Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-07-17 Thread Thiago Jung Bauermann


Hello,

Just going back to this question which I wasn't able to answer.

Thiago Jung Bauermann  writes:

> Michael S. Tsirkin  writes:
>
>> So far so good, but now a question:
>>
>> how are we handling guest address width limitations?
>> Is VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS subject to
>> guest address width limitations?
>> I am guessing we can make them so ...
>> This needs to be documented.
>
> I'm not sure. I will get back to you on this.

We don't have address width limitations between host and guest.

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-07-15 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Mon, Jul 15, 2019 at 07:03:03PM -0300, Thiago Jung Bauermann wrote:
>> 
>> Michael S. Tsirkin  writes:
>> 
>> > On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >> Michael S. Tsirkin  writes:
>> >>
>> >> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>> >> >>
>> >> >>
>> >> >> Michael S. Tsirkin  writes:
>> >> >>
>> >> >> > So this is what I would call this option:
>> >> >> >
>> >> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>> >> >> >
>> >> >> > and the explanation should state that all device
>> >> >> > addresses are translated by the platform to identical
>> >> >> > addresses.
>> >> >> >
>> >> >> > In fact this option then becomes more, not less restrictive
>> >> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
>> >> >> > by guest to only create identity mappings,
>> >> >> > and only before driver_ok is set.
>> >> >> > This option then would always be negotiated together with
>> >> >> > VIRTIO_F_ACCESS_PLATFORM.
>> >> >> >
>> >> >> > Host then must verify that
>> >> >> > 1. full 1:1 mappings are created before driver_ok
>> >> >> > or can we make sure this happens before features_ok?
>> >> >> > that would be ideal as we could require that features_ok fails
>> >> >> > 2. mappings are not modified between driver_ok and reset
>> >> >> > i guess attempts to change them will fail -
>> >> >> > possibly by causing a guest crash
>> >> >> > or some other kind of platform-specific error
>> >> >>
>> >> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but 
>> >> >> requiring
>> >> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
>> >> >> SLOF as I mentioned above, another is that we would be requiring all
>> >> >> guests running on the machine (secure guests or not, since we would use
>> >> >> the same configuration for all guests) to support it. But
>> >> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
>> >> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know 
>> >> >> about
>> >> >> it and wouldn't be able to use the device.
>> >> >
>> >> > OK and your target is to enable use with kernel drivers within
>> >> > guests, right?
>> >>
>> >> Right.
>> >>
>> >> > My question is, we are defining a new flag here, I guess old guests
>> >> > then do not set it. How does it help old guests? Or maybe it's
>> >> > not designed to ...
>> >>
>> >> Indeed. The idea is that QEMU can offer the flag, old guests can reject
>> >> it (or even new guests can reject it, if they decide not to convert into
>> >> secure VMs) and the feature negotiation will succeed with the flag
>> >> unset.
>> >
>> > OK. And then what does QEMU do? Assume guest is not encrypted I guess?
>> 
>> There's nothing different that QEMU needs to do, with or without the
>> flag. the perspective of the host, a secure guest and a regular guest
>> work the same way with respect to virtio.
>
> OK. So now let's get back to implementation. What will
> Linux guest driver do? It can't activate DMA API blindly since that
> will assume translation also works, right?

It can on pseries, because we always have a 1:1 window mapping the whole
guest memory.

> Or do we somehow limit it to just a specific platform?

Yes, we want to accept the new flag only on secure pseries guests.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-07-15 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Mon, Jul 15, 2019 at 05:29:06PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >>
>> >> Michael S. Tsirkin  writes:
>> >>
>> >> > So this is what I would call this option:
>> >> >
>> >> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>> >> >
>> >> > and the explanation should state that all device
>> >> > addresses are translated by the platform to identical
>> >> > addresses.
>> >> >
>> >> > In fact this option then becomes more, not less restrictive
>> >> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
>> >> > by guest to only create identity mappings,
>> >> > and only before driver_ok is set.
>> >> > This option then would always be negotiated together with
>> >> > VIRTIO_F_ACCESS_PLATFORM.
>> >> >
>> >> > Host then must verify that
>> >> > 1. full 1:1 mappings are created before driver_ok
>> >> > or can we make sure this happens before features_ok?
>> >> > that would be ideal as we could require that features_ok fails
>> >> > 2. mappings are not modified between driver_ok and reset
>> >> > i guess attempts to change them will fail -
>> >> > possibly by causing a guest crash
>> >> > or some other kind of platform-specific error
>> >>
>> >> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
>> >> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
>> >> SLOF as I mentioned above, another is that we would be requiring all
>> >> guests running on the machine (secure guests or not, since we would use
>> >> the same configuration for all guests) to support it. But
>> >> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
>> >> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
>> >> it and wouldn't be able to use the device.
>> >
>> > OK and your target is to enable use with kernel drivers within
>> > guests, right?
>>
>> Right.
>>
>> > My question is, we are defining a new flag here, I guess old guests
>> > then do not set it. How does it help old guests? Or maybe it's
>> > not designed to ...
>>
>> Indeed. The idea is that QEMU can offer the flag, old guests can reject
>> it (or even new guests can reject it, if they decide not to convert into
>> secure VMs) and the feature negotiation will succeed with the flag
>> unset.
>
> OK. And then what does QEMU do? Assume guest is not encrypted I guess?

There's nothing different that QEMU needs to do, with or without the
flag. the perspective of the host, a secure guest and a regular guest
work the same way with respect to virtio.

--
Thiago Jung Bauermann
IBM Linux Technology Center
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-07-15 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Sun, Jul 14, 2019 at 02:51:18AM -0300, Thiago Jung Bauermann wrote:
>>
>>
>> Michael S. Tsirkin  writes:
>>
>> > So this is what I would call this option:
>> >
>> > VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>> >
>> > and the explanation should state that all device
>> > addresses are translated by the platform to identical
>> > addresses.
>> >
>> > In fact this option then becomes more, not less restrictive
>> > than VIRTIO_F_ACCESS_PLATFORM - it's a promise
>> > by guest to only create identity mappings,
>> > and only before driver_ok is set.
>> > This option then would always be negotiated together with
>> > VIRTIO_F_ACCESS_PLATFORM.
>> >
>> > Host then must verify that
>> > 1. full 1:1 mappings are created before driver_ok
>> > or can we make sure this happens before features_ok?
>> > that would be ideal as we could require that features_ok fails
>> > 2. mappings are not modified between driver_ok and reset
>> > i guess attempts to change them will fail -
>> > possibly by causing a guest crash
>> > or some other kind of platform-specific error
>>
>> I think VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS is good, but requiring
>> it to be accompanied by ACCESS_PLATFORM can be a problem. One reason is
>> SLOF as I mentioned above, another is that we would be requiring all
>> guests running on the machine (secure guests or not, since we would use
>> the same configuration for all guests) to support it. But
>> ACCESS_PLATFORM is relatively recent so it's a bit early for that. For
>> instance, Ubuntu 16.04 LTS (which is still supported) doesn't know about
>> it and wouldn't be able to use the device.
>
> OK and your target is to enable use with kernel drivers within
> guests, right?

Right.

> My question is, we are defining a new flag here, I guess old guests
> then do not set it. How does it help old guests? Or maybe it's
> not designed to ...

Indeed. The idea is that QEMU can offer the flag, old guests can reject
it (or even new guests can reject it, if they decide not to convert into
secure VMs) and the feature negotiation will succeed with the flag
unset.

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-07-14 Thread Thiago Jung Bauermann



Michael S. Tsirkin  writes:

> On Thu, Jun 27, 2019 at 10:58:40PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >>
>> >> Michael S. Tsirkin  writes:
>> >>
>> >> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
>> >> >> I rephrased it in terms of address translation. What do you think of
>> >> >> this version? The flag name is slightly different too:
>> >> >>
>> >> >>
>> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same
>> >> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set,
>> >> >> with the exception that address translation is guaranteed to be
>> >> >> unnecessary when accessing memory addresses supplied to the device
>> >> >> by the driver. Which is to say, the device will always use physical
>> >> >> addresses matching addresses used by the driver (typically meaning
>> >> >> physical addresses used by the CPU) and not translated further. 
>> >> >> This
>> >> >> flag should be set by the guest if offered, but to allow for
>> >> >> backward-compatibility device implementations allow for it to be
>> >> >> left unset by the guest. It is an error to set both this flag and
>> >> >> VIRTIO_F_ACCESS_PLATFORM.
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged
>> >> > drivers. This is why devices fail when it's not negotiated.
>> >>
>> >> Just to clarify, what do you mean by unprivileged drivers? Is it drivers
>> >> implemented in guest userspace such as with VFIO? Or unprivileged in
>> >> some other sense such as needing to use bounce buffers for some reason?
>> >
>> > I had drivers in guest userspace in mind.
>>
>> Great. Thanks for clarifying.
>>
>> I don't think this flag would work for guest userspace drivers. Should I
>> add a note about that in the flag definition?
>
> I think you need to clarify access protection rules. Is it only
> translation that is bypassed or is any platform-specific
> protection mechanism bypassed too?

It is only translation. In a secure guest, if the device tries to access
a memory address that wasn't provided by the driver then the
architecture will deny that access. If the device accesses addresses
provided to it by the driver, then there's no protection mechanism or
translation to get in the way.

>> >> > This confuses me.
>> >> > If driver is unpriveledged then what happens with this flag?
>> >> > It can supply any address it wants. Will that corrupt kernel
>> >> > memory?
>> >>
>> >> Not needing address translation doesn't necessarily mean that there's no
>> >> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's
>> >> always an IOMMU present. And we also support VFIO drivers. The VFIO API
>> >> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls
>> >> to program the IOMMU.
>> >>
>> >> For our use case, we don't need address translation because we set up an
>> >> identity mapping in the IOMMU so that the device can use guest physical
>> >> addresses.
>
> OK so I think I am beginning to see it in a different light.  Right now the 
> specific
> platform creates an identity mapping. That in turn means DMA API can be
> fast - it does not need to do anything.  What you are looking for is a
> way to tell host it's an identity mapping - just as an optimization.
>
> Is that right?

Almost. Theoretically it is just an optimization. But in practice the
pseries boot firmware (SLOF) doesn't support IOMMU_PLATFORM so it's not
possible to boot a guest from a device with that flag set.

> So this is what I would call this option:
>
> VIRTIO_F_ACCESS_PLATFORM_IDENTITY_ADDRESS
>
> and the explanation should state that all device
> addresses are translated by the platform to identical
> addresses.
>
> In fact this option then becomes more, not less restrictive
> than VIRTIO_F_ACCESS_PLATFORM - it's a promise
> by guest to only create identity mappings,
> and only before driver_ok is set.
> This option then would always be negotiated together with
> VIRTIO_F_ACCES

Re: [PATCH v5 1/8] s390/mm: force swiotlb for protected virtualization

2019-07-11 Thread Thiago Jung Bauermann


Hello Halil,

Halil Pasic  writes:

> On s390, protected virtualization guests have to use bounced I/O
> buffers.  That requires some plumbing.
>
> Let us make sure, any device that uses DMA API with direct ops correctly
> is spared from the problems, that a hypervisor attempting I/O to a
> non-shared page would bring.
>
> Signed-off-by: Halil Pasic 
> Reviewed-by: Claudio Imbrenda 
> ---
>  arch/s390/Kconfig   |  4 +++
>  arch/s390/include/asm/mem_encrypt.h | 18 +++
>  arch/s390/mm/init.c | 47 +
>  3 files changed, 69 insertions(+)
>  create mode 100644 arch/s390/include/asm/mem_encrypt.h
>
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 109243fdb6ec..88d8355b7bf7 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -1,4 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
> +config ARCH_HAS_MEM_ENCRYPT
> +def_bool y
> +
>  config MMU
>   def_bool y
>  

ARCH_HAS_MEM_ENCRYPT is already defined in arch/Kconfig, so I think you
can just select it in config S390 like you do with SWIOTLB.

> @@ -187,6 +190,7 @@ config S390
>   select VIRT_CPU_ACCOUNTING
>   select ARCH_HAS_SCALED_CPUTIME
>   select HAVE_NMI
> + select SWIOTLB
>  
>  
>  config SCHED_OMIT_FRAME_POINTER

-- 
Thiago Jung Bauermann
IBM Linux Technology Center
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-06-27 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Mon, Jun 03, 2019 at 10:13:59PM -0300, Thiago Jung Bauermann wrote:
>>
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
>> >> I rephrased it in terms of address translation. What do you think of
>> >> this version? The flag name is slightly different too:
>> >>
>> >>
>> >> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same
>> >> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set,
>> >> with the exception that address translation is guaranteed to be
>> >> unnecessary when accessing memory addresses supplied to the device
>> >> by the driver. Which is to say, the device will always use physical
>> >> addresses matching addresses used by the driver (typically meaning
>> >> physical addresses used by the CPU) and not translated further. This
>> >> flag should be set by the guest if offered, but to allow for
>> >> backward-compatibility device implementations allow for it to be
>> >> left unset by the guest. It is an error to set both this flag and
>> >> VIRTIO_F_ACCESS_PLATFORM.
>> >
>> >
>> > OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged
>> > drivers. This is why devices fail when it's not negotiated.
>>
>> Just to clarify, what do you mean by unprivileged drivers? Is it drivers
>> implemented in guest userspace such as with VFIO? Or unprivileged in
>> some other sense such as needing to use bounce buffers for some reason?
>
> I had drivers in guest userspace in mind.

Great. Thanks for clarifying.

I don't think this flag would work for guest userspace drivers. Should I
add a note about that in the flag definition?

>> > This confuses me.
>> > If driver is unpriveledged then what happens with this flag?
>> > It can supply any address it wants. Will that corrupt kernel
>> > memory?
>>
>> Not needing address translation doesn't necessarily mean that there's no
>> IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's
>> always an IOMMU present. And we also support VFIO drivers. The VFIO API
>> for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls
>> to program the IOMMU.
>>
>> For our use case, we don't need address translation because we set up an
>> identity mapping in the IOMMU so that the device can use guest physical
>> addresses.
>
> And can it access any guest physical address?

Sorry, I was mistaken. We do support VFIO in guests but not for virtio
devices, only for regular PCI devices. In which case they will use
address translation.

>> If the guest kernel is concerned that an unprivileged driver could
>> jeopardize its integrity it should not negotiate this feature flag.
>
> Unfortunately flag negotiation is done through config space
> and so can be overwritten by the driver.

Ok, so the guest kernel has to forbid VFIO access on devices where this
flag is advertised.

>> Perhaps there should be a note about this in the flag definition? This
>> concern is platform-dependant though. I don't believe it's an issue in
>> pseries.
>
> Again ACCESS_PLATFORM has a pretty open definition. It does actually
> say it's all up to the platform.
>
> Specifically how will VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION be
> implemented portably? virtio has no portable way to know
> whether DMA API bypasses translation.

The fact that VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION is set
communicates that knowledge to virtio. There is a shared understanding
between the guest and the host about what this flag being set means.

--
Thiago Jung Bauermann
IBM Linux Technology Center
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-06-03 Thread Thiago Jung Bauermann



Michael S. Tsirkin  writes:

> On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
>> I rephrased it in terms of address translation. What do you think of
>> this version? The flag name is slightly different too:
>>
>>
>> VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION This feature has the same
>> meaning as VIRTIO_F_ACCESS_PLATFORM both when set and when not set,
>> with the exception that address translation is guaranteed to be
>> unnecessary when accessing memory addresses supplied to the device
>> by the driver. Which is to say, the device will always use physical
>> addresses matching addresses used by the driver (typically meaning
>> physical addresses used by the CPU) and not translated further. This
>> flag should be set by the guest if offered, but to allow for
>> backward-compatibility device implementations allow for it to be
>> left unset by the guest. It is an error to set both this flag and
>> VIRTIO_F_ACCESS_PLATFORM.
>
>
> OK so VIRTIO_F_ACCESS_PLATFORM is designed to allow unpriveledged
> drivers. This is why devices fail when it's not negotiated.

Just to clarify, what do you mean by unprivileged drivers? Is it drivers
implemented in guest userspace such as with VFIO? Or unprivileged in
some other sense such as needing to use bounce buffers for some reason?

> This confuses me.
> If driver is unpriveledged then what happens with this flag?
> It can supply any address it wants. Will that corrupt kernel
> memory?

Not needing address translation doesn't necessarily mean that there's no
IOMMU. On powerpc we don't use VIRTIO_F_ACCESS_PLATFORM but there's
always an IOMMU present. And we also support VFIO drivers. The VFIO API
for pseries (sPAPR section in Documentation/vfio.txt) has extra ioctls
to program the IOMMU.

For our use case, we don't need address translation because we set up an
identity mapping in the IOMMU so that the device can use guest physical
addresses.

If the guest kernel is concerned that an unprivileged driver could
jeopardize its integrity it should not negotiate this feature flag.
Perhaps there should be a note about this in the flag definition? This
concern is platform-dependant though. I don't believe it's an issue in
pseries.

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-26 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Wed, Apr 24, 2019 at 10:01:56PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >> Michael S. Tsirkin  writes:
>> >>
>> >> > On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote:
>> >> >>
>> >> >> Michael S. Tsirkin  writes:
>> >> >>
>> >> >> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann 
>> >> >> > wrote:
>> >> >> >> >From what I understand of the ACCESS_PLATFORM definition, the host 
>> >> >> >> >will
>> >> >> >> only ever try to access memory addresses that are supplied to it by 
>> >> >> >> the
>> >> >> >> guest, so all of the secure guest memory that the host cares about 
>> >> >> >> is
>> >> >> >> accessible:
>> >> >> >>
>> >> >> >> If this feature bit is set to 0, then the device has same 
>> >> >> >> access to
>> >> >> >> memory addresses supplied to it as the driver has. In 
>> >> >> >> particular,
>> >> >> >> the device will always use physical addresses matching addresses
>> >> >> >> used by the driver (typically meaning physical addresses used 
>> >> >> >> by the
>> >> >> >> CPU) and not translated further, and can access any address 
>> >> >> >> supplied
>> >> >> >> to it by the driver. When clear, this overrides any
>> >> >> >> platform-specific description of whether device access is 
>> >> >> >> limited or
>> >> >> >> translated in any way, e.g. whether an IOMMU may be present.
>> >> >> >>
>> >> >> >> All of the above is true for POWER guests, whether they are secure
>> >> >> >> guests or not.
>> >> >> >>
>> >> >> >> Or are you saying that a virtio device may want to access memory
>> >> >> >> addresses that weren't supplied to it by the driver?
>> >> >> >
>> >> >> > Your logic would apply to IOMMUs as well.  For your mode, there are
>> >> >> > specific encrypted memory regions that driver has access to but 
>> >> >> > device
>> >> >> > does not. that seems to violate the constraint.
>> >> >>
>> >> >> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that
>> >> >> the device can ignore the IOMMU for all practical purposes I would
>> >> >> indeed say that the logic would apply to IOMMUs as well. :-)
>> >> >>
>> >> >> I guess I'm still struggling with the purpose of signalling to the
>> >> >> driver that the host may not have access to memory addresses that it
>> >> >> will never try to access.
>> >> >
>> >> > For example, one of the benefits is to signal to host that driver does
>> >> > not expect ability to access all memory. If it does, host can
>> >> > fail initialization gracefully.
>> >>
>> >> But why would the ability to access all memory be necessary or even
>> >> useful? When would the host access memory that the driver didn't tell it
>> >> to access?
>> >
>> > When I say all memory I mean even memory not allowed by the IOMMU.
>>
>> Yes, but why? How is that memory relevant?
>
> It's relevant when driver is not trusted to only supply correct
> addresses. The feature was originally designed to support userspace
> drivers within guests.

Ah, thanks for clarifying. I don't think that's a problem in our case.
If the guest provides an incorrect address, the hardware simply won't
allow the host to access it.

>> >> >> > Another idea is maybe something like virtio-iommu?
>> >> >>
>> >> >> You mean, have legacy guests use virtio-iommu to request an IOMMU
>> >> >> bypass? If so, it's an interesting idea for new guests but it doesn't
>> >> >> help with guests that are out today in the field, which don't have A
>> >> >> virtio-iommu driver.

Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-24 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Wed, Apr 17, 2019 at 06:42:00PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote:
>> >>
>> >> Michael S. Tsirkin  writes:
>> >>
>> >> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote:
>> >> >> >From what I understand of the ACCESS_PLATFORM definition, the host 
>> >> >> >will
>> >> >> only ever try to access memory addresses that are supplied to it by the
>> >> >> guest, so all of the secure guest memory that the host cares about is
>> >> >> accessible:
>> >> >>
>> >> >> If this feature bit is set to 0, then the device has same access to
>> >> >> memory addresses supplied to it as the driver has. In particular,
>> >> >> the device will always use physical addresses matching addresses
>> >> >> used by the driver (typically meaning physical addresses used by 
>> >> >> the
>> >> >> CPU) and not translated further, and can access any address 
>> >> >> supplied
>> >> >> to it by the driver. When clear, this overrides any
>> >> >> platform-specific description of whether device access is limited 
>> >> >> or
>> >> >> translated in any way, e.g. whether an IOMMU may be present.
>> >> >>
>> >> >> All of the above is true for POWER guests, whether they are secure
>> >> >> guests or not.
>> >> >>
>> >> >> Or are you saying that a virtio device may want to access memory
>> >> >> addresses that weren't supplied to it by the driver?
>> >> >
>> >> > Your logic would apply to IOMMUs as well.  For your mode, there are
>> >> > specific encrypted memory regions that driver has access to but device
>> >> > does not. that seems to violate the constraint.
>> >>
>> >> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that
>> >> the device can ignore the IOMMU for all practical purposes I would
>> >> indeed say that the logic would apply to IOMMUs as well. :-)
>> >>
>> >> I guess I'm still struggling with the purpose of signalling to the
>> >> driver that the host may not have access to memory addresses that it
>> >> will never try to access.
>> >
>> > For example, one of the benefits is to signal to host that driver does
>> > not expect ability to access all memory. If it does, host can
>> > fail initialization gracefully.
>>
>> But why would the ability to access all memory be necessary or even
>> useful? When would the host access memory that the driver didn't tell it
>> to access?
>
> When I say all memory I mean even memory not allowed by the IOMMU.

Yes, but why? How is that memory relevant?

>> >> >> >> > But the name "sev_active" makes me scared because at least AMD 
>> >> >> >> > guys who
>> >> >> >> > were doing the sensible thing and setting ACCESS_PLATFORM
>> >> >> >>
>> >> >> >> My understanding is, AMD guest-platform knows in advance that their
>> >> >> >> guest will run in secure mode and hence sets the flag at the time 
>> >> >> >> of VM
>> >> >> >> instantiation. Unfortunately we dont have that luxury on our 
>> >> >> >> platforms.
>> >> >> >
>> >> >> > Well you do have that luxury. It looks like that there are existing
>> >> >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy
>> >> >> > with how that path is slow. So you are trying to optimize for
>> >> >> > them by clearing ACCESS_PLATFORM and then you have lost ability
>> >> >> > to invoke DMA API.
>> >> >> >
>> >> >> > For example if there was another flag just like ACCESS_PLATFORM
>> >> >> > just not yet used by anyone, you would be all fine using that right?
>> >> >>
>> >> >> Yes, a new flag sounds like a great idea. What about the definition
>> >> >> below?
>> >> >>
>> >> >> VIRTIO_F_ACCESS_PLATFORM_NO_IOMMU This fe

Re: [PATCH v7 0/7] Add virtio-iommu driver

2019-04-19 Thread Thiago Jung Bauermann

Hello Jean-Philippe,

Jean-Philippe Brucker  writes:
> Makes sense, though I think other virtio devices have been developed a
> little more organically: device and driver code got upstreamed first,
> and then the specification describing their interface got merged into
> the standard. For example I believe that code for crypto, input and GPU
> devices were upstreamed long before the specification was merged. Once
> an implementation is upstream, the interface is expected to be
> backward-compatible (all subsequent changes are introduced using feature
> bits).
>
> So I've been working with this process in mind, also described by Jens
> at KVM forum 2017 [3]:
> (1) Reserve a device ID, and get that merged into virtio (ID 23 for
> virtio-iommu was reserved last year)
> (2) Open-source an implementation (this driver and Eric's device)
> (3) Formalize and upstream the device specification
>
> But I get that some overlap between (2) and (3) would have been better.
> So far the spec document has been reviewed mainly from the IOMMU point
> of view, and might require more changes to be in line with the other
> virtio devices -- hopefully just wording changes. I'll kick off step
> (3), but I think the virtio folks are a bit busy with finalizing the 1.1
> spec so I expect it to take a while.

I read v0.9 of the spec and have some minor comments, hope this is a
good place to send them:

1. In section 2.6.2, one reads

If the VIRTIO_IOMMU_F_INPUT_RANGE feature is offered and the range
described by fields virt_start and virt_end doesn’t fit in the range
described by input_range, the device MAY set status to VIRTIO_-
IOMMU_S_RANGE and ignore the request.

Shouldn't int say "If the VIRTIO_IOMMU_F_INPUT_RANGE feature is
negotiated" instead?

2. There's a typo at the end of section 2.6.5:

The VIRTIO_IOMMU_MAP_F_MMIO flag is a memory type rather than a
protection lag.

s/lag/flag/

3. In section 3.1.2.1.1, the viommu compatible field says "virtio,mmio".
Shouldn't it say "virtio,mmio-iommu" instead, to be consistent with
"virtio,pci-iommu"?

4. There's a typo in section 3.3:

A host bridge may limit the input address space – transaction
accessing some addresses won’t reach the physical IOMMU.

s/transaction/transactions/

I also have one last comment which you may freely ignore, considering
it's clearly just personal opinion and also considering that the
specification is mature at this point: it specifies memory ranges by
specifying start and end addresses. My experience has been that this is
error prone, leading to confusion and bugs regarding whether the end
address is inclusive or exclusive. I tend to prefer expressing memory
ranges by specifying a start address and a length, which eliminates
ambiguity.

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v7 0/7] Add virtio-iommu driver

2019-04-19 Thread Thiago Jung Bauermann
 making a small module that
>> > loads early and pokes at the IOMMU sufficiently to get the data about
>> > which devices use the IOMMU out of it using standard virtio config
>> > space.  IIUC it's claimed to be impossible without messy changes to the
>> > boot sequence.
>> >
>> > If I succeed at least on some platforms I'll ask that this design is
>> > worked into this device, minimizing info that goes through DT/ACPI.  If
>> > I see I can't make it in time to meet the next merge window, I plan
>> > merging the existing patches using DT (barring surprises).
>> >
>> > As I only have a very small amount of time to spend on this attempt, If
>> > someone else wants to try doing that in parallel, that would be great!

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-19 Thread Thiago Jung Bauermann


Hello Michael,

Michael S. Tsirkin  writes:

> On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
>>
>> Fixing address of powerpc mailing list.
>>
>> Thiago Jung Bauermann  writes:
>>
>> > Hello,
>> >
>> > With Christoph's rework of the DMA API that recently landed, the patch
>> > below is the only change needed in virtio to make it work in a POWER
>> > secure guest under the ultravisor.
>> >
>> > The other change we need (making sure the device's dma_map_ops is NULL
>> > so that the dma-direct/swiotlb code is used) can be made in
>> > powerpc-specific code.
>> >
>> > Of course, I also have patches (soon to be posted as RFC) which hook up
>> >  to the powerpc secure guest support code.
>> >
>> > What do you think?
>> >
>> > From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
>> > From: Thiago Jung Bauermann 
>> > Date: Thu, 24 Jan 2019 22:08:02 -0200
>> > Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
>> >
>> > The host can't access the guest memory when it's encrypted, so using
>> > regular memory pages for the ring isn't an option. Go through the DMA API.
>> >
>> > Signed-off-by: Thiago Jung Bauermann 
>
> Well I think this will come back to bite us (witness xen which is now
> reworking precisely this path - but at least they aren't to blame, xen
> came before ACCESS_PLATFORM).
>
> I also still think the right thing would have been to set
> ACCESS_PLATFORM for all systems where device can't access all memory.

I understand. The problem with that approach for us is that because we
don't know which guests will become secure guests and which will remain
regular guests, QEMU would need to offer ACCESS_PLATFORM to all guests.

And the problem with that is that for QEMU on POWER, having
ACCESS_PLATFORM turned off means that it can bypass the IOMMU for the
device (which makes sense considering that the name of the flag was
IOMMU_PLATFORM). And we need that for regular guests to avoid
performance degradation.

So while ACCESS_PLATFORM solves our problems for secure guests, we can't
turn it on by default because we can't affect legacy systems. Doing so
would penalize existing systems that can access all memory. They would
all have to unnecessarily go through address translations, and take a
performance hit.

The semantics of ACCESS_PLATFORM assume that the hypervisor/QEMU knows
in advance - right when the VM is instantiated - that it will not have
access to all guest memory. Unfortunately that assumption is subtly
broken on our secure-platform. The hypervisor/QEMU realizes that the
platform is going secure only *after the VM is instantiated*. It's the
kernel running in the VM that determines that it wants to switch the
platform to secure-mode.

Another way of looking at this issue which also explains our reluctance
is that the only difference between a secure guest and a regular guest
(at least regarding virtio) is that the former uses swiotlb while the
latter doens't. And from the device's point of view they're
indistinguishable. It can't tell one guest that is using swiotlb from
one that isn't. And that implies that secure guest vs regular guest
isn't a virtio interface issue, it's "guest internal affairs". So
there's no reason to reflect that in the feature flags.

That said, we still would like to arrive at a proper design for this
rather than add yet another hack if we can avoid it. So here's another
proposal: considering that the dma-direct code (in kernel/dma/direct.c)
automatically uses swiotlb when necessary (thanks to Christoph's recent
DMA work), would it be ok to replace virtio's own direct-memory code
that is used in the !ACCESS_PLATFORM case with the dma-direct code? That
way we'll get swiotlb even with !ACCESS_PLATFORM, and virtio will get a
code cleanup (replace open-coded stuff with calls to existing
infrastructure).

> But I also think I don't have the energy to argue about power secure
> guest anymore.  So be it for power secure guest since the involved
> engineers disagree with me.  Hey I've been wrong in the past ;).

Yeah, it's been a difficult discussion. Thanks for still engaging!
I honestly thought that this patch was a good solution (if the guest has
encrypted memory it means that the DMA API needs to be used), but I can
see where you are coming from. As I said, we'd like to arrive at a good
solution if possible.

> But the name "sev_active" makes me scared because at least AMD guys who
> were doing the sensible thing and setting ACCESS_PLATFORM

My understanding is, AMD guest-platform knows in advance that their
guest will run in secure mode and hence sets the flag at the time of VM
instantiation. Unfortunately 

Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-19 Thread Thiago Jung Bauermann


Christoph Hellwig  writes:

> On Tue, Jan 29, 2019 at 09:36:08PM -0500, Michael S. Tsirkin wrote:
>> This has been discussed ad nauseum. virtio is all about compatibility.
>> Losing a couple of lines of code isn't worth breaking working setups.
>> People that want "just use DMA API no tricks" now have the option.
>> Setting a flag in a feature bit map is literally a single line
>> of code in the hypervisor. So stop pushing for breaking working
>> legacy setups and just fix it in the right place.
>
> I agree with the legacy aspect.  What I am missing is an extremely
> strong wording that says you SHOULD always set this flag for new
> hosts, including an explanation why.

My understanding of ACCESS_PLATFORM is that it means "this device will
behave in all aspects like a regular device attached to this bus". Is
that it? Therefore it should be set because it's the sane thing to do?

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-19 Thread Thiago Jung Bauermann


Hello,

With Christoph's rework of the DMA API that recently landed, the patch
below is the only change needed in virtio to make it work in a POWER
secure guest under the ultravisor.

The other change we need (making sure the device's dma_map_ops is NULL
so that the dma-direct/swiotlb code is used) can be made in
powerpc-specific code.

Of course, I also have patches (soon to be posted as RFC) which hook up
 to the powerpc secure guest support code.

What do you think?

>From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann 
Date: Thu, 24 Jan 2019 22:08:02 -0200
Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

The host can't access the guest memory when it's encrypted, so using
regular memory pages for the ring isn't an option. Go through the DMA API.

Signed-off-by: Thiago Jung Bauermann 
---
 drivers/virtio/virtio_ring.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..321a27075380 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
 * not work without an even larger kludge.  Instead, enable
 * the DMA API if we're a Xen guest, which at least allows
 * all of the sensible Xen configurations to work correctly.
+*
+* Also, if guest memory is encrypted the host can't access
+* it directly. In this case, we'll need to use the DMA API.
 */
-   if (xen_domain())
+   if (xen_domain() || sev_active())
return true;

return false;

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-19 Thread Thiago Jung Bauermann


Fixing address of powerpc mailing list.

Thiago Jung Bauermann  writes:

> Hello,
>
> With Christoph's rework of the DMA API that recently landed, the patch
> below is the only change needed in virtio to make it work in a POWER
> secure guest under the ultravisor.
>
> The other change we need (making sure the device's dma_map_ops is NULL
> so that the dma-direct/swiotlb code is used) can be made in
> powerpc-specific code.
>
> Of course, I also have patches (soon to be posted as RFC) which hook up
>  to the powerpc secure guest support code.
>
> What do you think?
>
> From d0629a36a75c678b4a72b853f8f7f8c17eedd6b3 Mon Sep 17 00:00:00 2001
> From: Thiago Jung Bauermann 
> Date: Thu, 24 Jan 2019 22:08:02 -0200
> Subject: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
>
> The host can't access the guest memory when it's encrypted, so using
> regular memory pages for the ring isn't an option. Go through the DMA API.
>
> Signed-off-by: Thiago Jung Bauermann 
> ---
>  drivers/virtio/virtio_ring.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..321a27075380 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -259,8 +259,11 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>* not work without an even larger kludge.  Instead, enable
>* the DMA API if we're a Xen guest, which at least allows
>* all of the sensible Xen configurations to work correctly.
> +  *
> +  * Also, if guest memory is encrypted the host can't access
> +  * it directly. In this case, we'll need to use the DMA API.
>*/
> - if (xen_domain())
> + if (xen_domain() || sev_active())
>   return true;
>
>   return false;


-- 
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-17 Thread Thiago Jung Bauermann


David Gibson  writes:

> On Sat, Mar 23, 2019 at 05:01:35PM -0400, Michael S. Tsirkin wrote:
>> On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote:
>> > Michael S. Tsirkin  writes:
> [snip]
>> > >> > Is there any justification to doing that beyond someone putting
>> > >> > out slow code in the past?
>> > >>
>> > >> The definition of the ACCESS_PLATFORM flag is generic and captures the
>> > >> notion of memory access restrictions for the device. Unfortunately, on
>> > >> powerpc pSeries guests it also implies that the IOMMU is turned on
>> > >
>> > > IIUC that's really because on pSeries IOMMU is *always* turned on.
>> > > Platform has no way to say what you want it to say
>> > > which is bypass the iommu for the specific device.
>> >
>> > Yes, that's correct. pSeries guests running on KVM are in a gray area
>> > where theoretically they use an IOMMU but in practice KVM ignores it.
>> > It's unfortunate but it's the reality on the ground today. :-/
>
> Um.. I'm not sure what you mean by this.  As far as I'm concerned
> there is always a guest-visible (paravirtualized) IOMMU, and that will
> be backed onto the host IOMMU when necessary.

There is, but vhost will ignore it and directly map the guest memory
when ACCESS_PLATFORM (the flag previously known as IOMMU_PLATFORM) isn't
set. From QEMU's hw/virtio/vhost.c:

static int vhost_dev_has_iommu(struct vhost_dev *dev)
{
VirtIODevice *vdev = dev->vdev;

return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
}

static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
      hwaddr *plen, int is_write)
{
if (!vhost_dev_has_iommu(dev)) {
return cpu_physical_memory_map(addr, plen, is_write);
} else {
return (void *)(uintptr_t)addr;
}
}

--
Thiago Jung Bauermann
IBM Linux Technology Center

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-04-17 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote:
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote:
>> >> >From what I understand of the ACCESS_PLATFORM definition, the host will
>> >> only ever try to access memory addresses that are supplied to it by the
>> >> guest, so all of the secure guest memory that the host cares about is
>> >> accessible:
>> >>
>> >> If this feature bit is set to 0, then the device has same access to
>> >> memory addresses supplied to it as the driver has. In particular,
>> >> the device will always use physical addresses matching addresses
>> >> used by the driver (typically meaning physical addresses used by the
>> >> CPU) and not translated further, and can access any address supplied
>> >> to it by the driver. When clear, this overrides any
>> >> platform-specific description of whether device access is limited or
>> >> translated in any way, e.g. whether an IOMMU may be present.
>> >>
>> >> All of the above is true for POWER guests, whether they are secure
>> >> guests or not.
>> >>
>> >> Or are you saying that a virtio device may want to access memory
>> >> addresses that weren't supplied to it by the driver?
>> >
>> > Your logic would apply to IOMMUs as well.  For your mode, there are
>> > specific encrypted memory regions that driver has access to but device
>> > does not. that seems to violate the constraint.
>>
>> Right, if there's a pre-configured 1:1 mapping in the IOMMU such that
>> the device can ignore the IOMMU for all practical purposes I would
>> indeed say that the logic would apply to IOMMUs as well. :-)
>>
>> I guess I'm still struggling with the purpose of signalling to the
>> driver that the host may not have access to memory addresses that it
>> will never try to access.
>
> For example, one of the benefits is to signal to host that driver does
> not expect ability to access all memory. If it does, host can
> fail initialization gracefully.

But why would the ability to access all memory be necessary or even
useful? When would the host access memory that the driver didn't tell it
to access?

>> >> >> > But the name "sev_active" makes me scared because at least AMD guys 
>> >> >> > who
>> >> >> > were doing the sensible thing and setting ACCESS_PLATFORM
>> >> >>
>> >> >> My understanding is, AMD guest-platform knows in advance that their
>> >> >> guest will run in secure mode and hence sets the flag at the time of VM
>> >> >> instantiation. Unfortunately we dont have that luxury on our platforms.
>> >> >
>> >> > Well you do have that luxury. It looks like that there are existing
>> >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy
>> >> > with how that path is slow. So you are trying to optimize for
>> >> > them by clearing ACCESS_PLATFORM and then you have lost ability
>> >> > to invoke DMA API.
>> >> >
>> >> > For example if there was another flag just like ACCESS_PLATFORM
>> >> > just not yet used by anyone, you would be all fine using that right?
>> >>
>> >> Yes, a new flag sounds like a great idea. What about the definition
>> >> below?
>> >>
>> >> VIRTIO_F_ACCESS_PLATFORM_NO_IOMMU This feature has the same meaning as
>> >> VIRTIO_F_ACCESS_PLATFORM both when set and when not set, with the
>> >> exception that the IOMMU is explicitly defined to be off or bypassed
>> >> when accessing memory addresses supplied to the device by the
>> >> driver. This flag should be set by the guest if offered, but to
>> >> allow for backward-compatibility device implementations allow for it
>> >> to be left unset by the guest. It is an error to set both this flag
>> >> and VIRTIO_F_ACCESS_PLATFORM.
>> >
>> > It looks kind of narrow but it's an option.
>>
>> Great!
>>
>> > I wonder how we'll define what's an iommu though.
>>
>> Hm, it didn't occur to me it could be an issue. I'll try.

I rephrased it in terms of address translation. What do you think of
this version? The flag name is slightly different too:


VIRTIO_F_ACCESS_PLATFORM_NO_TRANSLATION 

Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-03-21 Thread Thiago Jung Bauermann


Michael S. Tsirkin  writes:

> On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote:
>> >> Another way of looking at this issue which also explains our reluctance
>> >> is that the only difference between a secure guest and a regular guest
>> >> (at least regarding virtio) is that the former uses swiotlb while the
>> >> latter doens't.
>> >
>> > But swiotlb is just one implementation. It's a guest internal thing. The
>> > issue is that memory isn't host accessible.
>>
>> >From what I understand of the ACCESS_PLATFORM definition, the host will
>> only ever try to access memory addresses that are supplied to it by the
>> guest, so all of the secure guest memory that the host cares about is
>> accessible:
>>
>> If this feature bit is set to 0, then the device has same access to
>> memory addresses supplied to it as the driver has. In particular,
>> the device will always use physical addresses matching addresses
>> used by the driver (typically meaning physical addresses used by the
>> CPU) and not translated further, and can access any address supplied
>> to it by the driver. When clear, this overrides any
>> platform-specific description of whether device access is limited or
>> translated in any way, e.g. whether an IOMMU may be present.
>>
>> All of the above is true for POWER guests, whether they are secure
>> guests or not.
>>
>> Or are you saying that a virtio device may want to access memory
>> addresses that weren't supplied to it by the driver?
>
> Your logic would apply to IOMMUs as well.  For your mode, there are
> specific encrypted memory regions that driver has access to but device
> does not. that seems to violate the constraint.

Right, if there's a pre-configured 1:1 mapping in the IOMMU such that
the device can ignore the IOMMU for all practical purposes I would
indeed say that the logic would apply to IOMMUs as well. :-)

I guess I'm still struggling with the purpose of signalling to the
driver that the host may not have access to memory addresses that it
will never try to access.

>> >> And from the device's point of view they're
>> >> indistinguishable. It can't tell one guest that is using swiotlb from
>> >> one that isn't. And that implies that secure guest vs regular guest
>> >> isn't a virtio interface issue, it's "guest internal affairs". So
>> >> there's no reason to reflect that in the feature flags.
>> >
>> > So don't. The way not to reflect that in the feature flags is
>> > to set ACCESS_PLATFORM.  Then you say *I don't care let platform device*.
>> >
>> >
>> > Without ACCESS_PLATFORM
>> > virtio has a very specific opinion about the security of the
>> > device, and that opinion is that device is part of the guest
>> > supervisor security domain.
>>
>> Sorry for being a bit dense, but not sure what "the device is part of
>> the guest supervisor security domain" means. In powerpc-speak,
>> "supervisor" is the operating system so perhaps that explains my
>> confusion. Are you saying that without ACCESS_PLATFORM, the guest
>> considers the host to be part of the guest operating system's security
>> domain?
>
> I think so. The spec says "device has same access as driver".

Ok, makes sense.

>> If so, does that have any other implication besides "the host
>> can access any address supplied to it by the driver"? If that is the
>> case, perhaps the definition of ACCESS_PLATFORM needs to be amended to
>> include that information because it's not part of the current
>> definition.
>>
>> >> > But the name "sev_active" makes me scared because at least AMD guys who
>> >> > were doing the sensible thing and setting ACCESS_PLATFORM
>> >>
>> >> My understanding is, AMD guest-platform knows in advance that their
>> >> guest will run in secure mode and hence sets the flag at the time of VM
>> >> instantiation. Unfortunately we dont have that luxury on our platforms.
>> >
>> > Well you do have that luxury. It looks like that there are existing
>> > guests that already acknowledge ACCESS_PLATFORM and you are not happy
>> > with how that path is slow. So you are trying to optimize for
>> > them by clearing ACCESS_PLATFORM and then you have lost ability
>> > to invoke DMA API.
>> >
>> > For example if there was another flag just like ACCESS_PLATFORM
>> > just not yet used by anyone, you would be all fi

Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted

2019-03-20 Thread Thiago Jung Bauermann


Hello Michael,

Sorry for the delay in responding. We had some internal discussions on
this.

Michael S. Tsirkin  writes:

> On Mon, Feb 04, 2019 at 04:14:20PM -0200, Thiago Jung Bauermann wrote:
>>
>> Hello Michael,
>>
>> Michael S. Tsirkin  writes:
>>
>> > On Tue, Jan 29, 2019 at 03:42:44PM -0200, Thiago Jung Bauermann wrote:
>> So while ACCESS_PLATFORM solves our problems for secure guests, we can't
>> turn it on by default because we can't affect legacy systems. Doing so
>> would penalize existing systems that can access all memory. They would
>> all have to unnecessarily go through address translations, and take a
>> performance hit.
>
> So as step one, you just give hypervisor admin an option to run legacy
> systems faster by blocking secure mode. I don't see why that is
> so terrible.

There are a few reasons why:

1. It's bad user experience to require people to fiddle with knobs for
obscure reasons if it's possible to design things such that they Just
Work.

2. "User" in this case can be a human directly calling QEMU, but could
also be libvirt or one of its users, or some other framework. This means
having to adjust and/or educate an open-ended number of people and
software. It's best avoided if possible.

3. The hypervisor admin and the admin of the guest system don't
necessarily belong to the same organization (e.g., cloud provider and
cloud customer), so there may be some friction when they need to
coordinate to get this right.

4. A feature of our design is that the guest may or may not decide to
"go secure" at boot time, so it's best not to depend on flags that may
or may not have been set at the time QEMU was started.

>> The semantics of ACCESS_PLATFORM assume that the hypervisor/QEMU knows
>> in advance - right when the VM is instantiated - that it will not have
>> access to all guest memory.
>
> Not quite. It just means that hypervisor can live with not having
> access to all memory. If platform wants to give it access
> to all memory that is quite all right.

Except that on powerpc it also means "there's an IOMMU present" and
there's no way to say "bypass IOMMU translation". :-/

>> Another way of looking at this issue which also explains our reluctance
>> is that the only difference between a secure guest and a regular guest
>> (at least regarding virtio) is that the former uses swiotlb while the
>> latter doens't.
>
> But swiotlb is just one implementation. It's a guest internal thing. The
> issue is that memory isn't host accessible.

>From what I understand of the ACCESS_PLATFORM definition, the host will
only ever try to access memory addresses that are supplied to it by the
guest, so all of the secure guest memory that the host cares about is
accessible:

If this feature bit is set to 0, then the device has same access to
memory addresses supplied to it as the driver has. In particular,
the device will always use physical addresses matching addresses
used by the driver (typically meaning physical addresses used by the
CPU) and not translated further, and can access any address supplied
to it by the driver. When clear, this overrides any
platform-specific description of whether device access is limited or
translated in any way, e.g. whether an IOMMU may be present.

All of the above is true for POWER guests, whether they are secure
guests or not.

Or are you saying that a virtio device may want to access memory
addresses that weren't supplied to it by the driver?

>> And from the device's point of view they're
>> indistinguishable. It can't tell one guest that is using swiotlb from
>> one that isn't. And that implies that secure guest vs regular guest
>> isn't a virtio interface issue, it's "guest internal affairs". So
>> there's no reason to reflect that in the feature flags.
>
> So don't. The way not to reflect that in the feature flags is
> to set ACCESS_PLATFORM.  Then you say *I don't care let platform device*.
>
>
> Without ACCESS_PLATFORM
> virtio has a very specific opinion about the security of the
> device, and that opinion is that device is part of the guest
> supervisor security domain.

Sorry for being a bit dense, but not sure what "the device is part of
the guest supervisor security domain" means. In powerpc-speak,
"supervisor" is the operating system so perhaps that explains my
confusion. Are you saying that without ACCESS_PLATFORM, the guest
considers the host to be part of the guest operating system's security
domain? If so, does that have any other implication besides "the host
can access any address supplied to it by the driver"? If that is the
case, perhaps the definition of ACCESS_PLATFORM needs to be amended to
include that information becau