mbox series

[QEMU,PATCHv2,0/8] Xen: support grant mappings.

Message ID 20231025212422.30371-1-vikram.garhwal@amd.com (mailing list archive)
Headers show
Series Xen: support grant mappings. | expand

Message

Vikram Garhwal Oct. 25, 2023, 9:24 p.m. UTC
Hi,
This patch series add support for grant mappings as a pseudo RAM region for Xen.

Enabling grant mappings patches(first 6) are written by Juergen in 2021.

QEMU Virtio device provides an emulated backends for Virtio frontned devices
in Xen.
Please set "iommu_platform=on" option when invoking QEMU. As this will set
VIRTIO_F_ACCESS_PLATFORM feature which will be used by virtio frontend in Xen
to know whether backend supports grants or not.

Changelog:
v1->v2:
    Split patch 2/7 to keep phymem.c changes in a separate.
    In patch "xen: add map and unmap callbacks for grant" add check for total
        allowed grant < XEN_MAX_VIRTIO_GRANTS.
    Fix formatting issues and re-based with master latest.

Regards,
Vikram
Juergen Gross (6):
  xen: when unplugging emulated devices skip virtio devices
  xen: add pseudo RAM region for grant mappings
  softmmu: let qemu_map_ram_ptr() use qemu_ram_ptr_length()
  xen: let xen_ram_addr_from_mapcache() return -1 in case of not found
    entry
  memory: add MemoryRegion map and unmap callbacks
  xen: add map and unmap callbacks for grant region

Vikram Garhwal (2):
  softmmu: physmem: Split ram_block_add()
  hw: arm: Add grant mapping.

 docs/system/i386/xen.rst        |   3 -
 hw/arm/xen_arm.c                |   3 +
 hw/i386/xen/xen-hvm.c           |   3 +
 hw/i386/xen/xen_platform.c      |  10 +-
 hw/xen/xen-hvm-common.c         |   4 +-
 hw/xen/xen-mapcache.c           | 214 ++++++++++++++++++++++++++++++--
 include/exec/memory.h           |  21 ++++
 include/exec/ram_addr.h         |   1 +
 include/hw/xen/xen-hvm-common.h |   2 +
 include/hw/xen/xen_pvdev.h      |   3 +
 include/sysemu/xen-mapcache.h   |   3 +
 system/physmem.c                | 181 ++++++++++++++++-----------
 12 files changed, 358 insertions(+), 90 deletions(-)

Comments

David Woodhouse Oct. 26, 2023, 4:12 p.m. UTC | #1
On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> Hi,
> This patch series add support for grant mappings as a pseudo RAM region for Xen.
> 
> Enabling grant mappings patches(first 6) are written by Juergen in 2021.
> 
> QEMU Virtio device provides an emulated backends for Virtio frontned devices
> in Xen.
> Please set "iommu_platform=on" option when invoking QEMU. As this will set
> VIRTIO_F_ACCESS_PLATFORM feature which will be used by virtio frontend in Xen
> to know whether backend supports grants or not.

I don't really understand what's going on here. The subject of the
cover letter certainly doesn't help me, because we *already* support
grant mappings under Xen, don't we?

I found
https://static.linaro.org/connect/lvc21/presentations/lvc21-314.pdf but
I think it's a bit out of date; the decision about how to handle grant
mappings for virtio devices is still 'TBD'.

Can you talk me through the process of what happens when a guest wants
to a virtio device to initiate 'DMA' to one of its pages? I assume it
starts by creating a grant mapping, and then taking the gntref and...
then what?

I don't see any changes to the virtio devices themselves in this
series; are we doing something that will make it work by magic? If so,
it might be useful to explain that magic...
Stefano Stabellini Oct. 26, 2023, 6:07 p.m. UTC | #2
On Thu, 26 Oct 2023, David Woodhouse wrote:
> On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > Hi,
> > This patch series add support for grant mappings as a pseudo RAM region for Xen.
> > 
> > Enabling grant mappings patches(first 6) are written by Juergen in 2021.
> > 
> > QEMU Virtio device provides an emulated backends for Virtio frontned devices
> > in Xen.
> > Please set "iommu_platform=on" option when invoking QEMU. As this will set
> > VIRTIO_F_ACCESS_PLATFORM feature which will be used by virtio frontend in Xen
> > to know whether backend supports grants or not.
> 
> I don't really understand what's going on here. The subject of the
> cover letter certainly doesn't help me, because we *already* support
> grant mappings under Xen, don't we?
> 
> I found
> https://static.linaro.org/connect/lvc21/presentations/lvc21-314.pdf but
> I think it's a bit out of date; the decision about how to handle grant
> mappings for virtio devices is still 'TBD'.

See this presentation:
https://www.youtube.com/watch?v=boRQ8UHc760

The patch series is for the guest (e.g. Linux) to use grants to share
memory with virtio devices. The plumbing was already done in Linux a
couple of years ago, but QEMU support for it is still missing.


> Can you talk me through the process of what happens when a guest wants
> to a virtio device to initiate 'DMA' to one of its pages? I assume it
> starts by creating a grant mapping, and then taking the gntref and...
> then what?

First the guest gets a grant reference for the page, then it uses it as
"dma address" to pass to the virtio device. The top bit (1ULL << 63) is
used to signal that the address in question is a grant address.

See in Linux:
drivers/xen/grant-dma-ops.c grant_to_dma, dma_to_grant,
xen_grant_dma_alloc, etc.


> I don't see any changes to the virtio devices themselves in this
> series; are we doing something that will make it work by magic? If so,
> it might be useful to explain that magic...

Yes something like that :-)
https://marc.info/?l=xen-devel&m=165419780304962&w=2

Vikram, I think it would be a good idea to submit a separate patch to
xen.git to add a document under docs/ to capture this.
David Woodhouse Oct. 26, 2023, 8:15 p.m. UTC | #3
On Thu, 2023-10-26 at 11:07 -0700, Stefano Stabellini wrote:
> On Thu, 26 Oct 2023, David Woodhouse wrote:
> > On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > > Hi,
> > > This patch series add support for grant mappings as a pseudo RAM region for Xen.
> > > 
> > > Enabling grant mappings patches(first 6) are written by Juergen in 2021.
> > > 
> > > QEMU Virtio device provides an emulated backends for Virtio frontned devices
> > > in Xen.
> > > Please set "iommu_platform=on" option when invoking QEMU. As this will set
> > > VIRTIO_F_ACCESS_PLATFORM feature which will be used by virtio frontend in Xen
> > > to know whether backend supports grants or not.
> > 
> > I don't really understand what's going on here. The subject of the
> > cover letter certainly doesn't help me, because we *already* support
> > grant mappings under Xen, don't we?
> > 
> > I found
> > https://static.linaro.org/connect/lvc21/presentations/lvc21-314.pdf but
> > I think it's a bit out of date; the decision about how to handle grant
> > mappings for virtio devices is still 'TBD'.
> 
> See this presentation:
> https://www.youtube.com/watch?v=boRQ8UHc760
> 
> The patch series is for the guest (e.g. Linux) to use grants to share
> memory with virtio devices. The plumbing was already done in Linux a
> couple of years ago, but QEMU support for it is still missing.

Thanks.

> > Can you talk me through the process of what happens when a guest wants
> > to a virtio device to initiate 'DMA' to one of its pages? I assume it
> > starts by creating a grant mapping, and then taking the gntref and...
> > then what?
> 
> First the guest gets a grant reference for the page, then it uses it as
> "dma address" to pass to the virtio device. The top bit (1ULL << 63) is
> used to signal that the address in question is a grant address.

OK, so the guest sets the top bit in the DMA address and that indicates
that this is no longer actually a guest physical address, but instead,
bits 62-12 are a (starting) grant ref. (And the address *within* the
page is still bits 0-11, assuming 4KiB pages).

An alternative way of implementing this on the QEMU side would just be
to teach the virtio mapping to recognise the "special" format with the
top bit set, and call qemu_xen_gnttab_map_refs() directly to get the
mapping?

This seems like a lot of code to replace that simpler option... is
there a massive performance win from doing it this way? Would we want
to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
make sense to introduce the simple version and *then* the optimisation,
with some clear benchmarking to show the win?

If we're just going to switch to grant mappings, why *aren't* we using
Xen PV device models on Arm anyway?

Or am I missing the point completely?
Stefano Stabellini Oct. 26, 2023, 8:36 p.m. UTC | #4
On Thu, 26 Oct 2023, David Woodhouse wrote:
> On Thu, 2023-10-26 at 11:07 -0700, Stefano Stabellini wrote:
> > On Thu, 26 Oct 2023, David Woodhouse wrote:
> > > On Wed, 2023-10-25 at 14:24 -0700, Vikram Garhwal wrote:
> > > > Hi,
> > > > This patch series add support for grant mappings as a pseudo RAM region for Xen.
> > > > 
> > > > Enabling grant mappings patches(first 6) are written by Juergen in 2021.
> > > > 
> > > > QEMU Virtio device provides an emulated backends for Virtio frontned devices
> > > > in Xen.
> > > > Please set "iommu_platform=on" option when invoking QEMU. As this will set
> > > > VIRTIO_F_ACCESS_PLATFORM feature which will be used by virtio frontend in Xen
> > > > to know whether backend supports grants or not.
> > > 
> > > I don't really understand what's going on here. The subject of the
> > > cover letter certainly doesn't help me, because we *already* support
> > > grant mappings under Xen, don't we?
> > > 
> > > I found
> > > https://static.linaro.org/connect/lvc21/presentations/lvc21-314.pdf but
> > > I think it's a bit out of date; the decision about how to handle grant
> > > mappings for virtio devices is still 'TBD'.
> > 
> > See this presentation:
> > https://www.youtube.com/watch?v=boRQ8UHc760
> > 
> > The patch series is for the guest (e.g. Linux) to use grants to share
> > memory with virtio devices. The plumbing was already done in Linux a
> > couple of years ago, but QEMU support for it is still missing.
> 
> Thanks.
> 
> > > Can you talk me through the process of what happens when a guest wants
> > > to a virtio device to initiate 'DMA' to one of its pages? I assume it
> > > starts by creating a grant mapping, and then taking the gntref and...
> > > then what?
> > 
> > First the guest gets a grant reference for the page, then it uses it as
> > "dma address" to pass to the virtio device. The top bit (1ULL << 63) is
> > used to signal that the address in question is a grant address.
> 
> OK, so the guest sets the top bit in the DMA address and that indicates
> that this is no longer actually a guest physical address, but instead,
> bits 62-12 are a (starting) grant ref. (And the address *within* the
> page is still bits 0-11, assuming 4KiB pages).
> 
> An alternative way of implementing this on the QEMU side would just be
> to teach the virtio mapping to recognise the "special" format with the
> top bit set, and call qemu_xen_gnttab_map_refs() directly to get the
> mapping?
> 
> This seems like a lot of code to replace that simpler option... is
> there a massive performance win from doing it this way? Would we want
> to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
> make sense to introduce the simple version and *then* the optimisation,
> with some clear benchmarking to show the win?

This is not done for performance but for safety (as in safety
certifications, ISO 26262, etc.). This is to enable unprivileged virtio
backends running in a DomU. By unprivileged I mean a virtio backend that
is unable to map arbitrary memory (the xenforeignmemory interface is
prohibited).

The goal is to run Xen on safety-critical systems such as cars,
industrial robots and more. In this configuration there is no
traditional Dom0 in the system at all. If you  would like to know more:
https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8


> If we're just going to switch to grant mappings, why *aren't* we using
> Xen PV device models on Arm anyway?

I think you mean Xen PV drivers. Yes we have been using them for a
while. Now we would also like to add the option of running virtio
devices but we can only do that if the virtio backend is unprivileged as
we have no Dom0 in the system.
David Woodhouse Oct. 26, 2023, 8:44 p.m. UTC | #5
On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
> 
> > This seems like a lot of code to replace that simpler option... is
> > there a massive performance win from doing it this way? Would we want
> > to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
> > make sense to introduce the simple version and *then* the optimisation,
> > with some clear benchmarking to show the win?
> 
> This is not done for performance but for safety (as in safety
> certifications, ISO 26262, etc.). This is to enable unprivileged virtio
> backends running in a DomU. By unprivileged I mean a virtio backend that
> is unable to map arbitrary memory (the xenforeignmemory interface is
> prohibited).
> 
> The goal is to run Xen on safety-critical systems such as cars,
> industrial robots and more. In this configuration there is no
> traditional Dom0 in the system at all. If you  would like to know more:
> https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8

Yeah, I understand why we're using grant mappings instead of just
directly having access via foreignmem mappings. That wasn't what I was
confused about.

What I haven't worked out is why we're implementing this through an
automatically-populated MemoryRegion in QEMU, rather than just using
grant mapping ops like we always have.

It seems like a lot of complexity just to avoid calling
qemu_xen_gnttab_map_refs() from the virtio backend.

And if we're going to use this magic region, are we going to start
using it for the Xen PV backends in QEMU too, when they want to map
grant refs?
Stefano Stabellini Oct. 26, 2023, 8:56 p.m. UTC | #6
On Thu, 26 Oct 2023, David Woodhouse wrote:
> On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
> > 
> > > This seems like a lot of code to replace that simpler option... is
> > > there a massive performance win from doing it this way? Would we want
> > > to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
> > > make sense to introduce the simple version and *then* the optimisation,
> > > with some clear benchmarking to show the win?
> > 
> > This is not done for performance but for safety (as in safety
> > certifications, ISO 26262, etc.). This is to enable unprivileged virtio
> > backends running in a DomU. By unprivileged I mean a virtio backend that
> > is unable to map arbitrary memory (the xenforeignmemory interface is
> > prohibited).
> > 
> > The goal is to run Xen on safety-critical systems such as cars,
> > industrial robots and more. In this configuration there is no
> > traditional Dom0 in the system at all. If you  would like to know more:
> > https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
> 
> Yeah, I understand why we're using grant mappings instead of just
> directly having access via foreignmem mappings. That wasn't what I was
> confused about.
> 
> What I haven't worked out is why we're implementing this through an
> automatically-populated MemoryRegion in QEMU, rather than just using
> grant mapping ops like we always have.
> 
> It seems like a lot of complexity just to avoid calling
> qemu_xen_gnttab_map_refs() from the virtio backend.

I think there are two questions here. One question is "Why do we need
all the new grant mapping code added to xen-mapcache.c in patch #7?
Can't we use qemu_xen_gnttab_map_refs() instead?"

Good question, I'll let Juergen and Vikram comment as original authors.

And the second question is where to call the grant mapping from (whether
the new code or the old qemu_xen_gnttab_map_refs code it doesn't
matter). It could be even simpler than calling it from the virtio
backends: we could just call it from the existing qemu_ram_ptr_length()
hook. See this discussion:
https://marc.info/?l=qemu-devel&m=169828434927778
Jürgen Groß Oct. 27, 2023, 5:27 a.m. UTC | #7
On 26.10.23 22:56, Stefano Stabellini wrote:
> On Thu, 26 Oct 2023, David Woodhouse wrote:
>> On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
>>>
>>>> This seems like a lot of code to replace that simpler option... is
>>>> there a massive performance win from doing it this way? Would we want
>>>> to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
>>>> make sense to introduce the simple version and *then* the optimisation,
>>>> with some clear benchmarking to show the win?
>>>
>>> This is not done for performance but for safety (as in safety
>>> certifications, ISO 26262, etc.). This is to enable unprivileged virtio
>>> backends running in a DomU. By unprivileged I mean a virtio backend that
>>> is unable to map arbitrary memory (the xenforeignmemory interface is
>>> prohibited).
>>>
>>> The goal is to run Xen on safety-critical systems such as cars,
>>> industrial robots and more. In this configuration there is no
>>> traditional Dom0 in the system at all. If you  would like to know more:
>>> https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
>>
>> Yeah, I understand why we're using grant mappings instead of just
>> directly having access via foreignmem mappings. That wasn't what I was
>> confused about.
>>
>> What I haven't worked out is why we're implementing this through an
>> automatically-populated MemoryRegion in QEMU, rather than just using
>> grant mapping ops like we always have.
>>
>> It seems like a lot of complexity just to avoid calling
>> qemu_xen_gnttab_map_refs() from the virtio backend.
> 
> I think there are two questions here. One question is "Why do we need
> all the new grant mapping code added to xen-mapcache.c in patch #7?
> Can't we use qemu_xen_gnttab_map_refs() instead?"

The main motivation was to _avoid_ having to change all the backends.

My implementation enables _all_ qemu based virtio backends to use grant
mappings. And if a new backend is added to qemu, there will be no change
required to make it work with grants.

> And the second question is where to call the grant mapping from (whether
> the new code or the old qemu_xen_gnttab_map_refs code it doesn't
> matter). It could be even simpler than calling it from the virtio
> backends: we could just call it from the existing qemu_ram_ptr_length()
> hook. See this discussion:
> https://marc.info/?l=qemu-devel&m=169828434927778

I wanted to be explicit where and when the mapping and unmapping are
happening. Adding the mapping to qemu_ram_ptr_length() would be probably
possible, but it would be quite hard to verify that no double mappings
are happening. And I had problems with that approach when qemu was setting
up the ring page access.

Adding a map() and an unmap() hook seemed to be the cleanest solution, even
if it needs more code churn. The qemu_ram_ptr_length() approach seemed to be
more like a hack than a solution.


Juergen
David Woodhouse Nov. 13, 2023, 8:24 p.m. UTC | #8
On Fri, 2023-10-27 at 07:27 +0200, Juergen Gross wrote:
> On 26.10.23 22:56, Stefano Stabellini wrote:
> > On Thu, 26 Oct 2023, David Woodhouse wrote:
> > > On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
> > > > 
> > > > > This seems like a lot of code to replace that simpler option... is
> > > > > there a massive performance win from doing it this way? Would we want
> > > > > to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
> > > > > make sense to introduce the simple version and *then* the optimisation,
> > > > > with some clear benchmarking to show the win?
> > > > 
> > > > This is not done for performance but for safety (as in safety
> > > > certifications, ISO 26262, etc.). This is to enable unprivileged virtio
> > > > backends running in a DomU. By unprivileged I mean a virtio backend that
> > > > is unable to map arbitrary memory (the xenforeignmemory interface is
> > > > prohibited).
> > > > 
> > > > The goal is to run Xen on safety-critical systems such as cars,
> > > > industrial robots and more. In this configuration there is no
> > > > traditional Dom0 in the system at all. If you  would like to know more:
> > > > https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
> > > 
> > > Yeah, I understand why we're using grant mappings instead of just
> > > directly having access via foreignmem mappings. That wasn't what I was
> > > confused about.
> > > 
> > > What I haven't worked out is why we're implementing this through an
> > > automatically-populated MemoryRegion in QEMU, rather than just using
> > > grant mapping ops like we always have.
> > > 
> > > It seems like a lot of complexity just to avoid calling
> > > qemu_xen_gnttab_map_refs() from the virtio backend.
> > 
> > I think there are two questions here. One question is "Why do we need
> > all the new grant mapping code added to xen-mapcache.c in patch #7?
> > Can't we use qemu_xen_gnttab_map_refs() instead?"
> 
> The main motivation was to _avoid_ having to change all the backends.
> 
> My implementation enables _all_ qemu based virtio backends to use grant
> mappings. And if a new backend is added to qemu, there will be no change
> required to make it work with grants.

I'm not really convinced I buy that. This is a lot of complexity, and
don't backends need to call an appropriate mapping function to map via
an IOMMU if it's present anyway? Make then call a helper where you can
do this in one place directly instead of through a fake MemoryRegion,
and you're done, surely?
Jürgen Groß Nov. 14, 2023, 6:19 a.m. UTC | #9
On 13.11.23 21:24, David Woodhouse wrote:
> On Fri, 2023-10-27 at 07:27 +0200, Juergen Gross wrote:
>> On 26.10.23 22:56, Stefano Stabellini wrote:
>>> On Thu, 26 Oct 2023, David Woodhouse wrote:
>>>> On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
>>>>>
>>>>>> This seems like a lot of code to replace that simpler option... is
>>>>>> there a massive performance win from doing it this way? Would we want
>>>>>> to use this trick for the Xen PV backends (qdisk, qnic) *too*? Might it
>>>>>> make sense to introduce the simple version and *then* the optimisation,
>>>>>> with some clear benchmarking to show the win?
>>>>>
>>>>> This is not done for performance but for safety (as in safety
>>>>> certifications, ISO 26262, etc.). This is to enable unprivileged virtio
>>>>> backends running in a DomU. By unprivileged I mean a virtio backend that
>>>>> is unable to map arbitrary memory (the xenforeignmemory interface is
>>>>> prohibited).
>>>>>
>>>>> The goal is to run Xen on safety-critical systems such as cars,
>>>>> industrial robots and more. In this configuration there is no
>>>>> traditional Dom0 in the system at all. If you  would like to know more:
>>>>> https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
>>>>
>>>> Yeah, I understand why we're using grant mappings instead of just
>>>> directly having access via foreignmem mappings. That wasn't what I was
>>>> confused about.
>>>>
>>>> What I haven't worked out is why we're implementing this through an
>>>> automatically-populated MemoryRegion in QEMU, rather than just using
>>>> grant mapping ops like we always have.
>>>>
>>>> It seems like a lot of complexity just to avoid calling
>>>> qemu_xen_gnttab_map_refs() from the virtio backend.
>>>
>>> I think there are two questions here. One question is "Why do we need
>>> all the new grant mapping code added to xen-mapcache.c in patch #7?
>>> Can't we use qemu_xen_gnttab_map_refs() instead?"
>>
>> The main motivation was to _avoid_ having to change all the backends.
>>
>> My implementation enables _all_ qemu based virtio backends to use grant
>> mappings. And if a new backend is added to qemu, there will be no change
>> required to make it work with grants.
> 
> I'm not really convinced I buy that. This is a lot of complexity, and
> don't backends need to call an appropriate mapping function to map via
> an IOMMU if it's present anyway? Make then call a helper where you can
> do this in one place directly instead of through a fake MemoryRegion,
> and you're done, surely?

That was tested with unmodified block and net backends in qemu.

Maybe I missed something, but I think the IOMMU accesses are _not_ covering
accesses to the virtio rings from qemu. And this is something you really
want for driver domains.


Juergen
Stefano Stabellini Nov. 14, 2023, 8:58 p.m. UTC | #10
On Tue, 14 Nov 2023, Juergen Gross wrote:
> On 13.11.23 21:24, David Woodhouse wrote:
> > On Fri, 2023-10-27 at 07:27 +0200, Juergen Gross wrote:
> > > On 26.10.23 22:56, Stefano Stabellini wrote:
> > > > On Thu, 26 Oct 2023, David Woodhouse wrote:
> > > > > On Thu, 2023-10-26 at 13:36 -0700, Stefano Stabellini wrote:
> > > > > > 
> > > > > > > This seems like a lot of code to replace that simpler option... is
> > > > > > > there a massive performance win from doing it this way? Would we
> > > > > > > want
> > > > > > > to use this trick for the Xen PV backends (qdisk, qnic) *too*?
> > > > > > > Might it
> > > > > > > make sense to introduce the simple version and *then* the
> > > > > > > optimisation,
> > > > > > > with some clear benchmarking to show the win?
> > > > > > 
> > > > > > This is not done for performance but for safety (as in safety
> > > > > > certifications, ISO 26262, etc.). This is to enable unprivileged
> > > > > > virtio
> > > > > > backends running in a DomU. By unprivileged I mean a virtio backend
> > > > > > that
> > > > > > is unable to map arbitrary memory (the xenforeignmemory interface is
> > > > > > prohibited).
> > > > > > 
> > > > > > The goal is to run Xen on safety-critical systems such as cars,
> > > > > > industrial robots and more. In this configuration there is no
> > > > > > traditional Dom0 in the system at all. If you  would like to know
> > > > > > more:
> > > > > > https://www.youtube.com/watch?v=tisljY6Bqv0&list=PLYyw7IQjL-zHtpYtMpFR3KYdRn0rcp5Xn&index=8
> > > > > 
> > > > > Yeah, I understand why we're using grant mappings instead of just
> > > > > directly having access via foreignmem mappings. That wasn't what I was
> > > > > confused about.
> > > > > 
> > > > > What I haven't worked out is why we're implementing this through an
> > > > > automatically-populated MemoryRegion in QEMU, rather than just using
> > > > > grant mapping ops like we always have.
> > > > > 
> > > > > It seems like a lot of complexity just to avoid calling
> > > > > qemu_xen_gnttab_map_refs() from the virtio backend.
> > > > 
> > > > I think there are two questions here. One question is "Why do we need
> > > > all the new grant mapping code added to xen-mapcache.c in patch #7?
> > > > Can't we use qemu_xen_gnttab_map_refs() instead?"
> > > 
> > > The main motivation was to _avoid_ having to change all the backends.
> > > 
> > > My implementation enables _all_ qemu based virtio backends to use grant
> > > mappings. And if a new backend is added to qemu, there will be no change
> > > required to make it work with grants.
> > 
> > I'm not really convinced I buy that. This is a lot of complexity, and
> > don't backends need to call an appropriate mapping function to map via
> > an IOMMU if it's present anyway? Make then call a helper where you can
> > do this in one place directly instead of through a fake MemoryRegion,
> > and you're done, surely?
> 
> That was tested with unmodified block and net backends in qemu.
> 
> Maybe I missed something, but I think the IOMMU accesses are _not_ covering
> accesses to the virtio rings from qemu. And this is something you really
> want for driver domains.

Hi Juergen, David,

I don't think we should use the IOMMU accesses and I don't think we
should change the virtio backends.

My preference would be to either use the patch as is, or (better) to
reuse the original Xen hooks already in place (qemu_ram_ptr_length and
xen_invalidate_map_cache_entry, see
https://marc.info/?l=qemu-devel&m=169828434927778).

Juergen did a good job at adding new hooks in a clean way, but from my
point of view I would still prefer to only have the two existing hooks
instead of having 4 (2 old, 2 new).