Re: [PATCH v10 44/59] hw/xen: Support mapping grant frames

[Paul Durrant]

On 14/02/2023 15:35, David Woodhouse wrote:
> On 13 February 2023 16:31:57 CET, Paul Durrant <xadimgnik@gmail.com> wrote:
> > On 01/02/2023 14:31, David Woodhouse wrote:
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > >
> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > > ---
> > >    hw/i386/kvm/xen_gnttab.c  | 74 ++++++++++++++++++++++++++++++++++++++-
> > >    hw/i386/kvm/xen_overlay.c |  2 +-
> > >    hw/i386/kvm/xen_overlay.h |  2 ++
> > >    3 files changed, 76 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/i386/kvm/xen_gnttab.c b/hw/i386/kvm/xen_gnttab.c
> > > index ef8857e50c..cd8c3ae60d 100644
> > > --- a/hw/i386/kvm/xen_gnttab.c
> > > +++ b/hw/i386/kvm/xen_gnttab.c
> > > @@ -37,13 +37,27 @@ OBJECT_DECLARE_SIMPLE_TYPE(XenGnttabState, XEN_GNTTAB)
> > >    #define XEN_PAGE_SHIFT 12
> > >    #define XEN_PAGE_SIZE (1ULL << XEN_PAGE_SHIFT)
> > >    +#define ENTRIES_PER_FRAME_V1 (XEN_PAGE_SIZE / sizeof(grant_entry_v1_t))
> > > +#define ENTRIES_PER_FRAME_V2 (XEN_PAGE_SIZE / sizeof(grant_entry_v2_t))
> > > +
> > >    struct XenGnttabState {
> > >        /*< private >*/
> > >        SysBusDevice busdev;
> > >        /*< public >*/
> > >    +    QemuMutex gnt_lock;
> > > +
> > >        uint32_t nr_frames;
> > >        uint32_t max_frames;
> > > +
> > > +    union {
> > > +        grant_entry_v1_t *v1;
> > > +        grant_entry_v2_t *v2;
> > > +    } entries;
> > > +
> >
> > If you want to have v2 support, don't you need status frames too?
>
> If/when we add v2 support we will need that, but not yet. Seemed harmless 
> enough to have the union with the right types from day one though.

For someone reading this code, they might expect support to be there. It 
also makes things a little more cumbersome. TBH I really can't see the 
need to ever support v2 so my preference would just be to avoid mention 
of it and pretend it was all a bad dream.

  Paul