Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/xen/xen-front-pgdir-shbuf.c b/drivers/xen/xen-front-pgdir-shbuf.c
index a959dee21134..fa2921d4fbfc 100644
--- a/drivers/xen/xen-front-pgdir-shbuf.c
+++ b/drivers/xen/xen-front-pgdir-shbuf.c
@@ -21,15 +21,6 @@
#include <xen/xen-front-pgdir-shbuf.h>
-#ifndef GRANT_INVALID_REF
-/*
- * FIXME: usage of grant reference 0 as invalid grant reference:
- * grant reference 0 is valid, but never exposed to a PV driver,
- * because of the fact it is already in use/reserved by the PV console.
- */
-#define GRANT_INVALID_REF 0
-#endif
-
/**
* This structure represents the structure of a shared page
* that contains grant references to the pages of the shared
@@ -83,7 +74,7 @@ grant_ref_t
xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf *buf)
{
if (!buf->grefs)
- return GRANT_INVALID_REF;
+ return INVALID_GRANT_REF;
return buf->grefs[0];
}
@@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct xen_front_pgdir_shbuf *buf)
int i;
for (i = 0; i < buf->num_grefs; i++)
- if (buf->grefs[i] != GRANT_INVALID_REF)
+ if (buf->grefs[i] != INVALID_GRANT_REF)
gnttab_end_foreign_access(buf->grefs[i], 0UL);
}
kfree(buf->grefs);
@@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct xen_front_pgdir_shbuf *buf)
}
/* Last page must say there is no more pages. */
page_dir = (struct xen_page_directory *)ptr;
- page_dir->gref_dir_next_page = GRANT_INVALID_REF;
+ page_dir->gref_dir_next_page = INVALID_GRANT_REF;
}
/**
@@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct xen_front_pgdir_shbuf *buf)
if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
to_copy = grefs_left;
- page_dir->gref_dir_next_page = GRANT_INVALID_REF;
+ page_dir->gref_dir_next_page = INVALID_GRANT_REF;
} else {
to_copy = XEN_NUM_GREFS_PER_PAGE;
page_dir->gref_dir_next_page = buf->grefs[i + 1];
--
2.34.1On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgross@suse.com> wrote:
Hello Juergen
[sorry for the possible format issue]
Instead of using a private macro for an invalid grant reference use
> the common one.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
> b/drivers/xen/xen-front-pgdir-shbuf.c
> index a959dee21134..fa2921d4fbfc 100644
> --- a/drivers/xen/xen-front-pgdir-shbuf.c
> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
> @@ -21,15 +21,6 @@
>
> #include <xen/xen-front-pgdir-shbuf.h>
>
> -#ifndef GRANT_INVALID_REF
> -/*
> - * FIXME: usage of grant reference 0 as invalid grant reference:
> - * grant reference 0 is valid, but never exposed to a PV driver,
> - * because of the fact it is already in use/reserved by the PV console.
> - */
> -#define GRANT_INVALID_REF 0
> -#endif
> -
> /**
> * This structure represents the structure of a shared page
> * that contains grant references to the pages of the shared
> @@ -83,7 +74,7 @@ grant_ref_t
> xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf *buf)
> {
> if (!buf->grefs)
> - return GRANT_INVALID_REF;
> + return INVALID_GRANT_REF;
>
> return buf->grefs[0];
> }
> @@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct
> xen_front_pgdir_shbuf *buf)
> int i;
>
> for (i = 0; i < buf->num_grefs; i++)
> - if (buf->grefs[i] != GRANT_INVALID_REF)
> + if (buf->grefs[i] != INVALID_GRANT_REF)
> gnttab_end_foreign_access(buf->grefs[i],
> 0UL);
> }
> kfree(buf->grefs);
> @@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
> }
> /* Last page must say there is no more pages. */
> page_dir = (struct xen_page_directory *)ptr;
> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
> }
>
> /**
> @@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
>
> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
> to_copy = grefs_left;
> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
>
I faced an issue with testing PV Sound with the current series.
root@salvator-x-h3-4x2g-xt-domu:~# aplay /media/MoodyLoop.wav
Playing WAVE '/media/MoodyLoop.wav' : Signed 16 bit Little Endian, Rate
44100 Hz, Stereo
(XEN) common/grant_table.c:1053:d1v2 Bad ref 0xffffffff for d6
Here we have an interesting situation. PV Sound frontend uses this
xen-front-pgdir-shbuf framework. Technically, this patch changes
page_dir->gref_dir_next_page (reference to the next page describing page
directory) from 0 to 0xffffffff here.
#define INVALID_GRANT_REF ((grant_ref_t)-1)
But according to the protocol (sndif.h), "0" means that there are no more
pages in the list and the user space backend expects only that value. So
receiving 0xffffffff it assumes there are pages in the list and trying to
process...
https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650
I think, the same is relevant to backend_fill_page_dir() as well.
} else {
> to_copy = XEN_NUM_GREFS_PER_PAGE;
> page_dir->gref_dir_next_page = buf->grefs[i + 1];
> --
> 2.34.1
>
>
>
--
Regards,
Oleksandr Tyshchenko
On 28.04.22 20:03, Oleksandr Tyshchenko wrote:
>
>
> On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgross@suse.com
> <mailto:jgross@suse.com>> wrote:
>
> Hello Juergen
>
> [sorry for the possible format issue]
>
> Instead of using a private macro for an invalid grant reference use
> the common one.
>
> Signed-off-by: Juergen Gross <jgross@suse.com <mailto:jgross@suse.com>>
> ---
> drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
> b/drivers/xen/xen-front-pgdir-shbuf.c
> index a959dee21134..fa2921d4fbfc 100644
> --- a/drivers/xen/xen-front-pgdir-shbuf.c
> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
> @@ -21,15 +21,6 @@
>
> #include <xen/xen-front-pgdir-shbuf.h>
>
> -#ifndef GRANT_INVALID_REF
> -/*
> - * FIXME: usage of grant reference 0 as invalid grant reference:
> - * grant reference 0 is valid, but never exposed to a PV driver,
> - * because of the fact it is already in use/reserved by the PV console.
> - */
> -#define GRANT_INVALID_REF 0
> -#endif
> -
> /**
> * This structure represents the structure of a shared page
> * that contains grant references to the pages of the shared
> @@ -83,7 +74,7 @@ grant_ref_t
> xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf *buf)
> {
> if (!buf->grefs)
> - return GRANT_INVALID_REF;
> + return INVALID_GRANT_REF;
>
> return buf->grefs[0];
> }
> @@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct
> xen_front_pgdir_shbuf *buf)
> int i;
>
> for (i = 0; i < buf->num_grefs; i++)
> - if (buf->grefs[i] != GRANT_INVALID_REF)
> + if (buf->grefs[i] != INVALID_GRANT_REF)
> gnttab_end_foreign_access(buf->grefs[i], 0UL);
> }
> kfree(buf->grefs);
> @@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
> }
> /* Last page must say there is no more pages. */
> page_dir = (struct xen_page_directory *)ptr;
> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
> }
>
> /**
> @@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
>
> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
> to_copy = grefs_left;
> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
>
>
> I faced an issue with testing PV Sound with the current series.
>
> root@salvator-x-h3-4x2g-xt-domu:~# aplay /media/MoodyLoop.wav
> Playing WAVE '/media/MoodyLoop.wav' : Signed 16 bit Little Endian, Rate 44100
> Hz, Stereo
> (XEN) common/grant_table.c:1053:d1v2 Bad ref 0xffffffff for d6
>
> Here we have an interesting situation. PV Sound frontend uses this
> xen-front-pgdir-shbuf framework. Technically, this patch changes
> page_dir->gref_dir_next_page (reference to the next page describing page
> directory) from 0 to 0xffffffff here.
> #define INVALID_GRANT_REF ((grant_ref_t)-1)
>
> But according to the protocol (sndif.h), "0" means that there are no more pages
> in the list and the user space backend expects only that value. So
> receiving 0xffffffff it assumes there are pages in the list and trying to
> process...
Hmm, that's unfortunate.
> https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650
> <https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650>
>
>
> I think, the same is relevant to backend_fill_page_dir() as well.
Thanks for finding this.
Juergen
Hello Juergen
On 28.04.22 21:03, Oleksandr Tyshchenko wrote:
>
>
> On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgross@suse.com
> <mailto:jgross@suse.com>> wrote:
>
> Hello Juergen
>
> [sorry for the possible format issue]
>
> Instead of using a private macro for an invalid grant reference use
> the common one.
>
> Signed-off-by: Juergen Gross <jgross@suse.com
> <mailto:jgross@suse.com>>
> ---
> drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
> 1 file changed, 4 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
> b/drivers/xen/xen-front-pgdir-shbuf.c
> index a959dee21134..fa2921d4fbfc 100644
> --- a/drivers/xen/xen-front-pgdir-shbuf.c
> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
> @@ -21,15 +21,6 @@
>
> #include <xen/xen-front-pgdir-shbuf.h>
>
> -#ifndef GRANT_INVALID_REF
> -/*
> - * FIXME: usage of grant reference 0 as invalid grant reference:
> - * grant reference 0 is valid, but never exposed to a PV driver,
> - * because of the fact it is already in use/reserved by the PV
> console.
> - */
> -#define GRANT_INVALID_REF 0
> -#endif
> -
> /**
> * This structure represents the structure of a shared page
> * that contains grant references to the pages of the shared
> @@ -83,7 +74,7 @@ grant_ref_t
> xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf
> *buf)
> {
> if (!buf->grefs)
> - return GRANT_INVALID_REF;
> + return INVALID_GRANT_REF;
>
> return buf->grefs[0];
> }
> @@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct
> xen_front_pgdir_shbuf *buf)
> int i;
>
> for (i = 0; i < buf->num_grefs; i++)
> - if (buf->grefs[i] != GRANT_INVALID_REF)
> + if (buf->grefs[i] != INVALID_GRANT_REF)
> gnttab_end_foreign_access(buf->grefs[i], 0UL);
> }
> kfree(buf->grefs);
> @@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
> }
> /* Last page must say there is no more pages. */
> page_dir = (struct xen_page_directory *)ptr;
> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
> }
>
> /**
> @@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
>
> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
> to_copy = grefs_left;
> - page_dir->gref_dir_next_page =
> GRANT_INVALID_REF;
> + page_dir->gref_dir_next_page =
> INVALID_GRANT_REF;
>
>
> I faced an issue with testing PV Sound with the current series.
>
> root@salvator-x-h3-4x2g-xt-domu:~# aplay /media/MoodyLoop.wav
> Playing WAVE '/media/MoodyLoop.wav' : Signed 16 bit Little Endian,
> Rate 44100 Hz, Stereo
> (XEN) common/grant_table.c:1053:d1v2 Bad ref 0xffffffff for d6
>
> Here we have an interesting situation. PV Sound frontend uses this
> xen-front-pgdir-shbuf framework. Technically, this patch changes
> page_dir->gref_dir_next_page (reference to the next page describing
> page directory) from 0 to 0xffffffff here.
> #define INVALID_GRANT_REF ((grant_ref_t)-1)
>
> But according to the protocol (sndif.h), "0" means that there are no
> more pages in the list and the user space backend expects only that
> value. So receiving 0xffffffff it assumes there are pages in the list
> and trying to process...
> https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650
>
>
> I think, the same is relevant to backend_fill_page_dir() as well.
In addition to what I said yesterday:
PV Display also uses this xen-front-pgdir-shbuf framework. It's protocol
(displif.h) also mentions the same as sndif.h if the context of
gref_dir_next_page:
* gref_dir_next_page - grant_ref_t, reference to the next page describing
* page directory. Must be 0 if there are no more pages in the list.
With that local change both PV devices work in my environment.
diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
b/drivers/xen/xen-front-pgdir-shbuf.c
index fa2921d..ad4a88e 100644
--- a/drivers/xen/xen-front-pgdir-shbuf.c
+++ b/drivers/xen/xen-front-pgdir-shbuf.c
@@ -346,7 +346,7 @@ static void backend_fill_page_dir(struct
xen_front_pgdir_shbuf *buf)
}
/* Last page must say there is no more pages. */
page_dir = (struct xen_page_directory *)ptr;
- page_dir->gref_dir_next_page = INVALID_GRANT_REF;
+ page_dir->gref_dir_next_page = 0;
}
/**
@@ -375,7 +375,7 @@ static void guest_fill_page_dir(struct
xen_front_pgdir_shbuf *buf)
if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
to_copy = grefs_left;
- page_dir->gref_dir_next_page = INVALID_GRANT_REF;
+ page_dir->gref_dir_next_page = 0;
} else {
to_copy = XEN_NUM_GREFS_PER_PAGE;
page_dir->gref_dir_next_page = buf->grefs[i + 1];
(END)
>
> } else {
> to_copy = XEN_NUM_GREFS_PER_PAGE;
> page_dir->gref_dir_next_page =
> buf->grefs[i + 1];
> --
> 2.34.1
>
>
>
>
> --
> Regards,
>
> Oleksandr Tyshchenko
--
Regards,
Oleksandr Tyshchenko
On 29.04.22 17:28, Oleksandr wrote:
>
> Hello Juergen
>
>
> On 28.04.22 21:03, Oleksandr Tyshchenko wrote:
>>
>>
>> On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgross@suse.com
>> <mailto:jgross@suse.com>> wrote:
>>
>> Hello Juergen
>>
>> [sorry for the possible format issue]
>>
>> Instead of using a private macro for an invalid grant reference use
>> the common one.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com
>> <mailto:jgross@suse.com>>
>> ---
>> drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
>> b/drivers/xen/xen-front-pgdir-shbuf.c
>> index a959dee21134..fa2921d4fbfc 100644
>> --- a/drivers/xen/xen-front-pgdir-shbuf.c
>> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
>> @@ -21,15 +21,6 @@
>>
>> #include <xen/xen-front-pgdir-shbuf.h>
>>
>> -#ifndef GRANT_INVALID_REF
>> -/*
>> - * FIXME: usage of grant reference 0 as invalid grant reference:
>> - * grant reference 0 is valid, but never exposed to a PV driver,
>> - * because of the fact it is already in use/reserved by the PV
>> console.
>> - */
>> -#define GRANT_INVALID_REF 0
>> -#endif
>> -
>> /**
>> * This structure represents the structure of a shared page
>> * that contains grant references to the pages of the shared
>> @@ -83,7 +74,7 @@ grant_ref_t
>> xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf
>> *buf)
>> {
>> if (!buf->grefs)
>> - return GRANT_INVALID_REF;
>> + return INVALID_GRANT_REF;
>>
>> return buf->grefs[0];
>> }
>> @@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct
>> xen_front_pgdir_shbuf *buf)
>> int i;
>>
>> for (i = 0; i < buf->num_grefs; i++)
>> - if (buf->grefs[i] != GRANT_INVALID_REF)
>> + if (buf->grefs[i] != INVALID_GRANT_REF)
>> gnttab_end_foreign_access(buf->grefs[i], 0UL);
>> }
>> kfree(buf->grefs);
>> @@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct
>> xen_front_pgdir_shbuf *buf)
>> }
>> /* Last page must say there is no more pages. */
>> page_dir = (struct xen_page_directory *)ptr;
>> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
>> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
>> }
>>
>> /**
>> @@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct
>> xen_front_pgdir_shbuf *buf)
>>
>> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
>> to_copy = grefs_left;
>> - page_dir->gref_dir_next_page =
>> GRANT_INVALID_REF;
>> + page_dir->gref_dir_next_page =
>> INVALID_GRANT_REF;
>>
>>
>> I faced an issue with testing PV Sound with the current series.
>>
>> root@salvator-x-h3-4x2g-xt-domu:~# aplay /media/MoodyLoop.wav
>> Playing WAVE '/media/MoodyLoop.wav' : Signed 16 bit Little Endian, Rate 44100
>> Hz, Stereo
>> (XEN) common/grant_table.c:1053:d1v2 Bad ref 0xffffffff for d6
>>
>> Here we have an interesting situation. PV Sound frontend uses this
>> xen-front-pgdir-shbuf framework. Technically, this patch changes
>> page_dir->gref_dir_next_page (reference to the next page describing page
>> directory) from 0 to 0xffffffff here.
>> #define INVALID_GRANT_REF ((grant_ref_t)-1)
>>
>> But according to the protocol (sndif.h), "0" means that there are no more
>> pages in the list and the user space backend expects only that value. So
>> receiving 0xffffffff it assumes there are pages in the list and trying to
>> process...
>> https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650
>>
>>
>>
>> I think, the same is relevant to backend_fill_page_dir() as well.
>
>
> In addition to what I said yesterday:
>
> PV Display also uses this xen-front-pgdir-shbuf framework. It's protocol
> (displif.h) also mentions the same as sndif.h if the context of gref_dir_next_page:
>
> * gref_dir_next_page - grant_ref_t, reference to the next page describing
> * page directory. Must be 0 if there are no more pages in the list.
>
>
> With that local change both PV devices work in my environment.
>
> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
> b/drivers/xen/xen-front-pgdir-shbuf.c
> index fa2921d..ad4a88e 100644
> --- a/drivers/xen/xen-front-pgdir-shbuf.c
> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
> @@ -346,7 +346,7 @@ static void backend_fill_page_dir(struct
> xen_front_pgdir_shbuf *buf)
> }
> /* Last page must say there is no more pages. */
> page_dir = (struct xen_page_directory *)ptr;
> - page_dir->gref_dir_next_page = INVALID_GRANT_REF;
> + page_dir->gref_dir_next_page = 0;
> }
>
> /**
> @@ -375,7 +375,7 @@ static void guest_fill_page_dir(struct xen_front_pgdir_shbuf
> *buf)
>
> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
> to_copy = grefs_left;
> - page_dir->gref_dir_next_page = INVALID_GRANT_REF;
> + page_dir->gref_dir_next_page = 0;
> } else {
> to_copy = XEN_NUM_GREFS_PER_PAGE;
> page_dir->gref_dir_next_page = buf->grefs[i + 1];
I think I'll introduce a proper define for that purpose.
Juergen
On 02.05.22 16:31, Juergen Gross wrote:
Hello Juergen
> On 29.04.22 17:28, Oleksandr wrote:
>>
>> Hello Juergen
>>
>>
>> On 28.04.22 21:03, Oleksandr Tyshchenko wrote:
>>>
>>>
>>> On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgross@suse.com
>>> <mailto:jgross@suse.com>> wrote:
>>>
>>> Hello Juergen
>>>
>>> [sorry for the possible format issue]
>>>
>>> Instead of using a private macro for an invalid grant reference use
>>> the common one.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com
>>> <mailto:jgross@suse.com>>
>>> ---
>>> drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
>>> 1 file changed, 4 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
>>> b/drivers/xen/xen-front-pgdir-shbuf.c
>>> index a959dee21134..fa2921d4fbfc 100644
>>> --- a/drivers/xen/xen-front-pgdir-shbuf.c
>>> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
>>> @@ -21,15 +21,6 @@
>>>
>>> #include <xen/xen-front-pgdir-shbuf.h>
>>>
>>> -#ifndef GRANT_INVALID_REF
>>> -/*
>>> - * FIXME: usage of grant reference 0 as invalid grant reference:
>>> - * grant reference 0 is valid, but never exposed to a PV driver,
>>> - * because of the fact it is already in use/reserved by the PV
>>> console.
>>> - */
>>> -#define GRANT_INVALID_REF 0
>>> -#endif
>>> -
>>> /**
>>> * This structure represents the structure of a shared page
>>> * that contains grant references to the pages of the shared
>>> @@ -83,7 +74,7 @@ grant_ref_t
>>> xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf
>>> *buf)
>>> {
>>> if (!buf->grefs)
>>> - return GRANT_INVALID_REF;
>>> + return INVALID_GRANT_REF;
>>>
>>> return buf->grefs[0];
>>> }
>>> @@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct
>>> xen_front_pgdir_shbuf *buf)
>>> int i;
>>>
>>> for (i = 0; i < buf->num_grefs; i++)
>>> - if (buf->grefs[i] != GRANT_INVALID_REF)
>>> + if (buf->grefs[i] != INVALID_GRANT_REF)
>>> gnttab_end_foreign_access(buf->grefs[i], 0UL);
>>> }
>>> kfree(buf->grefs);
>>> @@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct
>>> xen_front_pgdir_shbuf *buf)
>>> }
>>> /* Last page must say there is no more pages. */
>>> page_dir = (struct xen_page_directory *)ptr;
>>> - page_dir->gref_dir_next_page = GRANT_INVALID_REF;
>>> + page_dir->gref_dir_next_page = INVALID_GRANT_REF;
>>> }
>>>
>>> /**
>>> @@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct
>>> xen_front_pgdir_shbuf *buf)
>>>
>>> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
>>> to_copy = grefs_left;
>>> - page_dir->gref_dir_next_page =
>>> GRANT_INVALID_REF;
>>> + page_dir->gref_dir_next_page =
>>> INVALID_GRANT_REF;
>>>
>>>
>>> I faced an issue with testing PV Sound with the current series.
>>>
>>> root@salvator-x-h3-4x2g-xt-domu:~# aplay /media/MoodyLoop.wav
>>> Playing WAVE '/media/MoodyLoop.wav' : Signed 16 bit Little Endian,
>>> Rate 44100 Hz, Stereo
>>> (XEN) common/grant_table.c:1053:d1v2 Bad ref 0xffffffff for d6
>>>
>>> Here we have an interesting situation. PV Sound frontend uses this
>>> xen-front-pgdir-shbuf framework. Technically, this patch changes
>>> page_dir->gref_dir_next_page (reference to the next page describing
>>> page directory) from 0 to 0xffffffff here.
>>> #define INVALID_GRANT_REF ((grant_ref_t)-1)
>>>
>>> But according to the protocol (sndif.h), "0" means that there are no
>>> more pages in the list and the user space backend expects only that
>>> value. So receiving 0xffffffff it assumes there are pages in the
>>> list and trying to process...
>>> https://elixir.bootlin.com/linux/v5.18-rc4/source/include/xen/interface/io/sndif.h#L650
>>>
>>>
>>>
>>> I think, the same is relevant to backend_fill_page_dir() as well.
>>
>>
>> In addition to what I said yesterday:
>>
>> PV Display also uses this xen-front-pgdir-shbuf framework. It's
>> protocol (displif.h) also mentions the same as sndif.h if the context
>> of gref_dir_next_page:
>>
>> * gref_dir_next_page - grant_ref_t, reference to the next page
>> describing
>> * page directory. Must be 0 if there are no more pages in the list.
>>
>>
>> With that local change both PV devices work in my environment.
>>
>> diff --git a/drivers/xen/xen-front-pgdir-shbuf.c
>> b/drivers/xen/xen-front-pgdir-shbuf.c
>> index fa2921d..ad4a88e 100644
>> --- a/drivers/xen/xen-front-pgdir-shbuf.c
>> +++ b/drivers/xen/xen-front-pgdir-shbuf.c
>> @@ -346,7 +346,7 @@ static void backend_fill_page_dir(struct
>> xen_front_pgdir_shbuf *buf)
>> }
>> /* Last page must say there is no more pages. */
>> page_dir = (struct xen_page_directory *)ptr;
>> - page_dir->gref_dir_next_page = INVALID_GRANT_REF;
>> + page_dir->gref_dir_next_page = 0;
>> }
>>
>> /**
>> @@ -375,7 +375,7 @@ static void guest_fill_page_dir(struct
>> xen_front_pgdir_shbuf *buf)
>>
>> if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
>> to_copy = grefs_left;
>> - page_dir->gref_dir_next_page =
>> INVALID_GRANT_REF;
>> + page_dir->gref_dir_next_page = 0;
>> } else {
>> to_copy = XEN_NUM_GREFS_PER_PAGE;
>> page_dir->gref_dir_next_page = buf->grefs[i
>> + 1];
>
> I think I'll introduce a proper define for that purpose.
I think it would be the best option.
>
>
>
> Juergen
--
Regards,
Oleksandr Tyshchenko
© 2016 - 2026 Red Hat, Inc.