Re: [PATCH] virtio_ring: use DMA when memory encryption is active

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Michael S. Tsirkin]

On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
> virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
> 
> Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>


Sorry, no.
host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.

Not worth it to work around broken hosts.

Xen is an exception we carry around since it predates the
introduction of VIRTIO_F_ACCESS_PLATFORM.


> ---
>  drivers/virtio/virtio_ring.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index becc77697960..8c68c475ec21 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -12,6 +12,7 @@
>  #include <linux/hrtimer.h>
>  #include <linux/dma-mapping.h>
>  #include <xen/xen.h>
> +#include <linux/mem_encrypt.h>
>  
>  #ifdef DEBUG
>  /* For development, we want to crash whenever the ring is screwed. */
> @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>  	if (xen_domain())
>  		return true;
>  
> +	/* Memory encryption requires DMA */
> +	if (mem_encrypt_active())
> +		return true;
> +
>  	return false;
>  }
>  
> -- 
> 2.25.1

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

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Sebastian Hofmann]

> Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
> 
>  
> On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
> > virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
> > 
> > Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
> 
> 
> Sorry, no.
> host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
> 
> Not worth it to work around broken hosts.
> 
> Xen is an exception we carry around since it predates the
> introduction of VIRTIO_F_ACCESS_PLATFORM.
> 
> 

Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.

Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.

Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device? Because that would have been my next attempt.
I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.

> > ---
> >  drivers/virtio/virtio_ring.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index becc77697960..8c68c475ec21 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/hrtimer.h>
> >  #include <linux/dma-mapping.h>
> >  #include <xen/xen.h>
> > +#include <linux/mem_encrypt.h>
> >  
> >  #ifdef DEBUG
> >  /* For development, we want to crash whenever the ring is screwed. */
> > @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> >  	if (xen_domain())
> >  		return true;
> >  
> > +	/* Memory encryption requires DMA */
> > +	if (mem_encrypt_active())
> > +		return true;
> > +
> >  	return false;
> >  }
> >  
> > -- 
> > 2.25.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Jason Wang]

On 2020/10/23 下午5:00, Sebastian Hofmann wrote:
>> Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
>>
>>   
>> On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
>>> virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
>>>
>>> Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
>>
>> Sorry, no.
>> host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
>>
>> Not worth it to work around broken hosts.
>>
>> Xen is an exception we carry around since it predates the
>> introduction of VIRTIO_F_ACCESS_PLATFORM.
>>
>>
> Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.
>
> Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.
>
> Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device?


It's just because it has been implemented. In addition to implement 
IOTLB, the virtio-vsock-pci must advertise ATS capability as well.


> Because that would have been my next attempt.
> I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.


A question, is vIOMMU a must for making SEV work?

Thanks


>
>>> ---
>>>   drivers/virtio/virtio_ring.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>> index becc77697960..8c68c475ec21 100644
>>> --- a/drivers/virtio/virtio_ring.c
>>> +++ b/drivers/virtio/virtio_ring.c
>>> @@ -12,6 +12,7 @@
>>>   #include <linux/hrtimer.h>
>>>   #include <linux/dma-mapping.h>
>>>   #include <xen/xen.h>
>>> +#include <linux/mem_encrypt.h>
>>>   
>>>   #ifdef DEBUG
>>>   /* For development, we want to crash whenever the ring is screwed. */
>>> @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>>   	if (xen_domain())
>>>   		return true;
>>>   
>>> +	/* Memory encryption requires DMA */
>>> +	if (mem_encrypt_active())
>>> +		return true;
>>> +
>>>   	return false;
>>>   }
>>>   
>>> -- 
>>> 2.25.1

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

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Sebastian Hofmann]

> Jason Wang <jasowang@redhat.com> hat am 23.10.2020 11:10 geschrieben:
> 
>  
> On 2020/10/23 下午5:00, Sebastian Hofmann wrote:
> >> Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
> >>
> >>   
> >> On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
> >>> virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
> >>>
> >>> Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
> >>
> >> Sorry, no.
> >> host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
> >>
> >> Not worth it to work around broken hosts.
> >>
> >> Xen is an exception we carry around since it predates the
> >> introduction of VIRTIO_F_ACCESS_PLATFORM.
> >>
> >>
> > Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.
> >
> > Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.
> >
> > Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device?
> 
> 
> It's just because it has been implemented. In addition to implement 
> IOTLB, the virtio-vsock-pci must advertise ATS capability as well.
> 
> 
> > Because that would have been my next attempt.
> > I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.
> 
> 
> A question, is vIOMMU a must for making SEV work?

Based on the demo code from AMD where they just use "-device virtio-scsi-pci,id=scsi,disable-legacy=on,iommu_platform=true" and my understanding of IOMMU: no, I don't think so.

The ATS capability is only required for vIOMMU, right?

> 
> Thanks
> 
> 
> >
> >>> ---
> >>>   drivers/virtio/virtio_ring.c | 5 +++++
> >>>   1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> >>> index becc77697960..8c68c475ec21 100644
> >>> --- a/drivers/virtio/virtio_ring.c
> >>> +++ b/drivers/virtio/virtio_ring.c
> >>> @@ -12,6 +12,7 @@
> >>>   #include <linux/hrtimer.h>
> >>>   #include <linux/dma-mapping.h>
> >>>   #include <xen/xen.h>
> >>> +#include <linux/mem_encrypt.h>
> >>>   
> >>>   #ifdef DEBUG
> >>>   /* For development, we want to crash whenever the ring is screwed. */
> >>> @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> >>>   	if (xen_domain())
> >>>   		return true;
> >>>   
> >>> +	/* Memory encryption requires DMA */
> >>> +	if (mem_encrypt_active())
> >>> +		return true;
> >>> +
> >>>   	return false;
> >>>   }
> >>>   
> >>> -- 
> >>> 2.25.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Jason Wang]

On 2020/10/23 下午6:55, Sebastian Hofmann wrote:
>> Jason Wang <jasowang@redhat.com> hat am 23.10.2020 11:10 geschrieben:
>>
>>   
>> On 2020/10/23 下午5:00, Sebastian Hofmann wrote:
>>>> Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
>>>>
>>>>    
>>>> On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
>>>>> virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
>>>>>
>>>>> Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
>>>> Sorry, no.
>>>> host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
>>>>
>>>> Not worth it to work around broken hosts.
>>>>
>>>> Xen is an exception we carry around since it predates the
>>>> introduction of VIRTIO_F_ACCESS_PLATFORM.
>>>>
>>>>
>>> Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.
>>>
>>> Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.
>>>
>>> Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device?
>>
>> It's just because it has been implemented. In addition to implement
>> IOTLB, the virtio-vsock-pci must advertise ATS capability as well.
>>
>>
>>> Because that would have been my next attempt.
>>> I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.
>>
>> A question, is vIOMMU a must for making SEV work?
> Based on the demo code from AMD where they just use "-device virtio-scsi-pci,id=scsi,disable-legacy=on,iommu_platform=true" and my understanding of IOMMU: no, I don't think so.
>
> The ATS capability is only required for vIOMMU, right?


Yes, so if it doesn't require a vIOMMU, you don't need to implement 
Device IOTLB in vhost-vsock.

Setting iommu_platform should be sufficient to vhost-vsock or it could 
be a bug somewhere.

Thanks


>
>> Thanks
>>
>>
>>>>> ---
>>>>>    drivers/virtio/virtio_ring.c | 5 +++++
>>>>>    1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>>>>> index becc77697960..8c68c475ec21 100644
>>>>> --- a/drivers/virtio/virtio_ring.c
>>>>> +++ b/drivers/virtio/virtio_ring.c
>>>>> @@ -12,6 +12,7 @@
>>>>>    #include <linux/hrtimer.h>
>>>>>    #include <linux/dma-mapping.h>
>>>>>    #include <xen/xen.h>
>>>>> +#include <linux/mem_encrypt.h>
>>>>>    
>>>>>    #ifdef DEBUG
>>>>>    /* For development, we want to crash whenever the ring is screwed. */
>>>>> @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>>>>>    	if (xen_domain())
>>>>>    		return true;
>>>>>    
>>>>> +	/* Memory encryption requires DMA */
>>>>> +	if (mem_encrypt_active())
>>>>> +		return true;
>>>>> +
>>>>>    	return false;
>>>>>    }
>>>>>    
>>>>> -- 
>>>>> 2.25.1

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

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Michael S. Tsirkin]

On Fri, Oct 23, 2020 at 11:00:54AM +0200, Sebastian Hofmann wrote:
> > Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
> > 
> >  
> > On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
> > > virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
> > > 
> > > Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
> > 
> > 
> > Sorry, no.
> > host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
> > 
> > Not worth it to work around broken hosts.
> > 
> > Xen is an exception we carry around since it predates the
> > introduction of VIRTIO_F_ACCESS_PLATFORM.
> > 
> > 
> 
> Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.
> 
> Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.
> 
> Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device? Because that would have been my next attempt.
> I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.

Not sure I understand the problem. Are you using qemu? If so just add
iommu_platform=on and you are done.


> > > ---
> > >  drivers/virtio/virtio_ring.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index becc77697960..8c68c475ec21 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/hrtimer.h>
> > >  #include <linux/dma-mapping.h>
> > >  #include <xen/xen.h>
> > > +#include <linux/mem_encrypt.h>
> > >  
> > >  #ifdef DEBUG
> > >  /* For development, we want to crash whenever the ring is screwed. */
> > > @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > >  	if (xen_domain())
> > >  		return true;
> > >  
> > > +	/* Memory encryption requires DMA */
> > > +	if (mem_encrypt_active())
> > > +		return true;
> > > +
> > >  	return false;
> > >  }
> > >  
> > > -- 
> > > 2.25.1

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

Re: [PATCH] virtio_ring: use DMA when memory encryption is active

[Sebastian Hofmann]

> Michael S. Tsirkin <mst@redhat.com> hat am 23.10.2020 17:49 geschrieben:
> 
>  
> On Fri, Oct 23, 2020 at 11:00:54AM +0200, Sebastian Hofmann wrote:
> > > Michael S. Tsirkin <mst@redhat.com> hat am 22.10.2020 13:39 geschrieben:
> > > 
> > >  
> > > On Wed, Oct 21, 2020 at 05:14:25PM +0200, Sebastian Hofmann wrote:
> > > > virtio_ring does not work with active memory encryption because the host cannot read it. Fix this by enforcing the use of DMA which uses shared (unencrypted) memory pages.
> > > > 
> > > > Signed-off-by: Sebastian Hofmann <sebastian@kaemmelot.de>
> > > 
> > > 
> > > Sorry, no.
> > > host which can not access all of driver memory must set VIRTIO_F_ACCESS_PLATFORM.
> > > 
> > > Not worth it to work around broken hosts.
> > > 
> > > Xen is an exception we carry around since it predates the
> > > introduction of VIRTIO_F_ACCESS_PLATFORM.
> > > 
> > > 
> > 
> > Thanks for pointing out VIRTIO_F_ACCESS_PLATFORM which I was not aware of. Maybe that patch was a bit naïve.
> > 
> > Basically I'm looking for a way to use vsock with qemu on AMD SEV. When I try to use IOMMU for vsock I get an EOPNOTSUPP out of vhost_vsock_set_features.
> > 
> > Is there a reason why vhost_vsock_set_features doesn't use vhost_init_device_iotlb as done in the net device? Because that would have been my next attempt.
> > I would appreciate a short comment on this idea or a recommendation for another solution that is better than the patch below.
> 
> Not sure I understand the problem. Are you using qemu? If so just add
> iommu_platform=on and you are done.
> 

That would be nice, but once I set iommu_platform=on (using Linux 5.4 as host and guest, qemu 5.1.0):
qemu-system-x86_64 -enable-kvm -cpu host -machine q35 -nographic -no-user-config -nodefaults -serial stdio \
-global virtio-mmio.force-legacy=off \
-device vhost-vsock-pci,guest-cid=3,disable-legacy=on,iommu_platform=on \
-object sev-guest,id=sev0,cbitpos=47,reduced-phys-bits=5 \
-machine dump-guest-core=off,memory-encryption=sev0 \
[some more arguments...]

...
qemu-system-x86_64: vhost_set_features failed: Operation not supported (95)
qemu-system-x86_64: Error starting vhost: 95
...

Therefore my question if it would be enough to use vhost_init_device_iotlb instead of returning EOPNOTSUPP in vhost_vsock_set_features when VIRTIO_F_ACCESS_PLATFORM is passed. Equivalent to what I see in vhost_net_set_features.
Or maybe I'm missing something important?

> > > > ---
> > > >  drivers/virtio/virtio_ring.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > > 
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index becc77697960..8c68c475ec21 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -12,6 +12,7 @@
> > > >  #include <linux/hrtimer.h>
> > > >  #include <linux/dma-mapping.h>
> > > >  #include <xen/xen.h>
> > > > +#include <linux/mem_encrypt.h>
> > > >  
> > > >  #ifdef DEBUG
> > > >  /* For development, we want to crash whenever the ring is screwed. */
> > > > @@ -255,6 +256,10 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
> > > >  	if (xen_domain())
> > > >  		return true;
> > > >  
> > > > +	/* Memory encryption requires DMA */
> > > > +	if (mem_encrypt_active())
> > > > +		return true;
> > > > +
> > > >  	return false;
> > > >  }
> > > >  
> > > > -- 
> > > > 2.25.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization