On Thu, Apr 28, 2022 at 11:28 AM Juergen Gross <jgr...@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 <jgr...@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
