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

[David Woodhouse]

On 14 February 2023 16:40:11 CET, Paul Durrant <xadimgnik@gmail.com> wrote:
>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.

Works for me. I'll rip it out.