diff mbox series

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

Message ID 20230201143148.1744093-45-dwmw2@infradead.org
State New
Headers show
Series Xen HVM support under KVM | expand

Commit Message

David Woodhouse Feb. 1, 2023, 2:31 p.m. UTC 
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(-)

Comments

Paul Durrant Feb. 13, 2023, 3:31 p.m. UTC | #1 
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?

   Paul

> +    MemoryRegion gnt_frames;
> +    MemoryRegion *gnt_aliases;
> +    uint64_t *gnt_frame_gpas;
>   };
>
David Woodhouse Feb. 14, 2023, 3:35 p.m. UTC | #2 
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.
Paul Durrant Feb. 14, 2023, 3:40 p.m. UTC | #3 
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
David Woodhouse Feb. 14, 2023, 3:41 p.m. UTC | #4 
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.
diff mbox series

Patch

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;
+
+    MemoryRegion gnt_frames;
+    MemoryRegion *gnt_aliases;
+    uint64_t *gnt_frame_gpas;
 };
 
 struct XenGnttabState *xen_gnttab_singleton;
@@ -51,6 +65,7 @@  struct XenGnttabState *xen_gnttab_singleton;
 static void xen_gnttab_realize(DeviceState *dev, Error **errp)
 {
     XenGnttabState *s = XEN_GNTTAB(dev);
+    int i;
 
     if (xen_mode != XEN_EMULATE) {
         error_setg(errp, "Xen grant table support is for Xen emulation");
@@ -58,6 +73,38 @@  static void xen_gnttab_realize(DeviceState *dev, Error **errp)
     }
     s->nr_frames = 0;
     s->max_frames = kvm_xen_get_gnttab_max_frames();
+    memory_region_init_ram(&s->gnt_frames, OBJECT(dev), "xen:grant_table",
+                           XEN_PAGE_SIZE * s->max_frames, &error_abort);
+    memory_region_set_enabled(&s->gnt_frames, true);
+    s->entries.v1 = memory_region_get_ram_ptr(&s->gnt_frames);
+    memset(s->entries.v1, 0, XEN_PAGE_SIZE * s->max_frames);
+
+    /* Create individual page-sizes aliases for overlays */
+    s->gnt_aliases = (void *)g_new0(MemoryRegion, s->max_frames);
+    s->gnt_frame_gpas = (void *)g_new(uint64_t, s->max_frames);
+    for (i = 0; i < s->max_frames; i++) {
+        memory_region_init_alias(&s->gnt_aliases[i], OBJECT(dev),
+                                 NULL, &s->gnt_frames,
+                                 i * XEN_PAGE_SIZE, XEN_PAGE_SIZE);
+        s->gnt_frame_gpas[i] = INVALID_GPA;
+    }
+
+    qemu_mutex_init(&s->gnt_lock);
+
+    xen_gnttab_singleton = s;
+}
+
+static int xen_gnttab_post_load(void *opaque, int version_id)
+{
+    XenGnttabState *s = XEN_GNTTAB(opaque);
+    uint32_t i;
+
+    for (i = 0; i < s->nr_frames; i++) {
+        if (s->gnt_frame_gpas[i] != INVALID_GPA) {
+            xen_overlay_do_map_page(&s->gnt_aliases[i], s->gnt_frame_gpas[i]);
+        }
+    }
+    return 0;
 }
 
 static bool xen_gnttab_is_needed(void *opaque)
@@ -70,8 +117,11 @@  static const VMStateDescription xen_gnttab_vmstate = {
     .version_id = 1,
     .minimum_version_id = 1,
     .needed = xen_gnttab_is_needed,
+    .post_load = xen_gnttab_post_load,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(nr_frames, XenGnttabState),
+        VMSTATE_VARRAY_UINT32(gnt_frame_gpas, XenGnttabState, nr_frames, 0,
+                              vmstate_info_uint64, uint64_t),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -106,6 +156,28 @@  type_init(xen_gnttab_register_types)
 
 int xen_gnttab_map_page(uint64_t idx, uint64_t gfn)
 {
-    return -ENOSYS;
+    XenGnttabState *s = xen_gnttab_singleton;
+    uint64_t gpa = gfn << XEN_PAGE_SHIFT;
+
+    if (!s) {
+        return -ENOTSUP;
+    }
+
+    if (idx >= s->max_frames) {
+        return -EINVAL;
+    }
+
+    QEMU_IOTHREAD_LOCK_GUARD();
+    QEMU_LOCK_GUARD(&s->gnt_lock);
+
+    xen_overlay_do_map_page(&s->gnt_aliases[idx], gpa);
+
+    s->gnt_frame_gpas[idx] = gpa;
+
+    if (s->nr_frames <= idx) {
+        s->nr_frames = idx + 1;
+    }
+
+    return 0;
 }
 
diff --git a/hw/i386/kvm/xen_overlay.c b/hw/i386/kvm/xen_overlay.c
index 8685d87959..39fda1b72c 100644
--- a/hw/i386/kvm/xen_overlay.c
+++ b/hw/i386/kvm/xen_overlay.c
@@ -49,7 +49,7 @@  struct XenOverlayState {
 
 struct XenOverlayState *xen_overlay_singleton;
 
-static void xen_overlay_do_map_page(MemoryRegion *page, uint64_t gpa)
+void xen_overlay_do_map_page(MemoryRegion *page, uint64_t gpa)
 {
     /*
      * Xen allows guests to map the same page as many times as it likes
diff --git a/hw/i386/kvm/xen_overlay.h b/hw/i386/kvm/xen_overlay.h
index 5c46a0b036..75ecb6b359 100644
--- a/hw/i386/kvm/xen_overlay.h
+++ b/hw/i386/kvm/xen_overlay.h
@@ -21,4 +21,6 @@  int xen_sync_long_mode(void);
 int xen_set_long_mode(bool long_mode);
 bool xen_is_long_mode(void);
 
+void xen_overlay_do_map_page(MemoryRegion *page, uint64_t gpa);
+
 #endif /* QEMU_XEN_OVERLAY_H */