diff mbox series

[for-6.0,v5,12/13] securable guest memory: Alter virtio default properties for protected guests

Message ID 20201204054415.579042-13-david@gibson.dropbear.id.au (mailing list archive)
State New, archived
Headers show
Series Generalize memory encryption models | expand

Commit Message

David Gibson Dec. 4, 2020, 5:44 a.m. UTC
The default behaviour for virtio devices is not to use the platforms normal
DMA paths, but instead to use the fact that it's running in a hypervisor
to directly access guest memory.  That doesn't work if the guest's memory
is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.

So, if a securable guest memory mechanism is enabled, then apply the
iommu_platform=on option so it will go through normal DMA mechanisms.
Those will presumably have some way of marking memory as shared with
the hypervisor or hardware so that DMA will work.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/core/machine.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Christian Borntraeger Dec. 4, 2020, 8:10 a.m. UTC | #1
On 04.12.20 06:44, David Gibson wrote:
> The default behaviour for virtio devices is not to use the platforms normal
> DMA paths, but instead to use the fact that it's running in a hypervisor
> to directly access guest memory.  That doesn't work if the guest's memory
> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> 
> So, if a securable guest memory mechanism is enabled, then apply the
> iommu_platform=on option so it will go through normal DMA mechanisms.
> Those will presumably have some way of marking memory as shared with
> the hypervisor or hardware so that DMA will work.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/machine.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index a67a27d03c..d16273d75d 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -28,6 +28,8 @@
>  #include "hw/mem/nvdimm.h"
>  #include "migration/vmstate.h"
>  #include "exec/securable-guest-memory.h"
> +#include "hw/virtio/virtio.h"
> +#include "hw/virtio/virtio-pci.h"
>  
>  GlobalProperty hw_compat_5_1[] = {
>      { "vhost-scsi", "num_queues", "1"},
> @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
>           * areas.
>           */
>          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
> +
> +        /*
> +         * Virtio devices can't count on directly accessing guest
> +         * memory, so they need iommu_platform=on to use normal DMA
> +         * mechanisms.  That requires also disabling legacy virtio
> +         * support for those virtio pci devices which allow it.
> +         */
> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> +                                   "on", true);
> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> +                                   "on", false);

I have not followed all the history (sorry). Should we also set iommu_platform
for virtio-ccw? Halil?
Cornelia Huck Dec. 4, 2020, 8:17 a.m. UTC | #2
On Fri, 4 Dec 2020 09:10:36 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 04.12.20 06:44, David Gibson wrote:
> > The default behaviour for virtio devices is not to use the platforms normal
> > DMA paths, but instead to use the fact that it's running in a hypervisor
> > to directly access guest memory.  That doesn't work if the guest's memory
> > is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> > 
> > So, if a securable guest memory mechanism is enabled, then apply the
> > iommu_platform=on option so it will go through normal DMA mechanisms.
> > Those will presumably have some way of marking memory as shared with
> > the hypervisor or hardware so that DMA will work.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/core/machine.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index a67a27d03c..d16273d75d 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -28,6 +28,8 @@
> >  #include "hw/mem/nvdimm.h"
> >  #include "migration/vmstate.h"
> >  #include "exec/securable-guest-memory.h"
> > +#include "hw/virtio/virtio.h"
> > +#include "hw/virtio/virtio-pci.h"
> >  
> >  GlobalProperty hw_compat_5_1[] = {
> >      { "vhost-scsi", "num_queues", "1"},
> > @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
> >           * areas.
> >           */
> >          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
> > +
> > +        /*
> > +         * Virtio devices can't count on directly accessing guest
> > +         * memory, so they need iommu_platform=on to use normal DMA
> > +         * mechanisms.  That requires also disabling legacy virtio
> > +         * support for those virtio pci devices which allow it.
> > +         */
> > +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> > +                                   "on", true);
> > +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> > +                                   "on", false);  
> 
> I have not followed all the history (sorry). Should we also set iommu_platform
> for virtio-ccw? Halil?
> 

That line should add iommu_platform for all virtio devices, shouldn't
it?
Christian Borntraeger Dec. 4, 2020, 8:29 a.m. UTC | #3
On 04.12.20 09:17, Cornelia Huck wrote:
> On Fri, 4 Dec 2020 09:10:36 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 04.12.20 06:44, David Gibson wrote:
>>> The default behaviour for virtio devices is not to use the platforms normal
>>> DMA paths, but instead to use the fact that it's running in a hypervisor
>>> to directly access guest memory.  That doesn't work if the guest's memory
>>> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
>>>
>>> So, if a securable guest memory mechanism is enabled, then apply the
>>> iommu_platform=on option so it will go through normal DMA mechanisms.
>>> Those will presumably have some way of marking memory as shared with
>>> the hypervisor or hardware so that DMA will work.
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> ---
>>>  hw/core/machine.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index a67a27d03c..d16273d75d 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -28,6 +28,8 @@
>>>  #include "hw/mem/nvdimm.h"
>>>  #include "migration/vmstate.h"
>>>  #include "exec/securable-guest-memory.h"
>>> +#include "hw/virtio/virtio.h"
>>> +#include "hw/virtio/virtio-pci.h"
>>>  
>>>  GlobalProperty hw_compat_5_1[] = {
>>>      { "vhost-scsi", "num_queues", "1"},
>>> @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
>>>           * areas.
>>>           */
>>>          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
>>> +
>>> +        /*
>>> +         * Virtio devices can't count on directly accessing guest
>>> +         * memory, so they need iommu_platform=on to use normal DMA
>>> +         * mechanisms.  That requires also disabling legacy virtio
>>> +         * support for those virtio pci devices which allow it.
>>> +         */
>>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
>>> +                                   "on", true);
>>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
>>> +                                   "on", false);  
>>
>> I have not followed all the history (sorry). Should we also set iommu_platform
>> for virtio-ccw? Halil?
>>
> 
> That line should add iommu_platform for all virtio devices, shouldn't
> it?

Yes, sorry. Was misreading that with the line above.
Halil Pasic Dec. 4, 2020, 2:43 p.m. UTC | #4
On Fri, 4 Dec 2020 09:29:59 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> 
> 
> On 04.12.20 09:17, Cornelia Huck wrote:
> > On Fri, 4 Dec 2020 09:10:36 +0100
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > 
> >> On 04.12.20 06:44, David Gibson wrote:
> >>> The default behaviour for virtio devices is not to use the platforms normal
> >>> DMA paths, but instead to use the fact that it's running in a hypervisor
> >>> to directly access guest memory.  That doesn't work if the guest's memory
> >>> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> >>>
> >>> So, if a securable guest memory mechanism is enabled, then apply the
> >>> iommu_platform=on option so it will go through normal DMA mechanisms.
> >>> Those will presumably have some way of marking memory as shared with
> >>> the hypervisor or hardware so that DMA will work.
> >>>
> >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >>> ---
> >>>  hw/core/machine.c | 13 +++++++++++++
> >>>  1 file changed, 13 insertions(+)
> >>>
> >>> diff --git a/hw/core/machine.c b/hw/core/machine.c
> >>> index a67a27d03c..d16273d75d 100644
> >>> --- a/hw/core/machine.c
> >>> +++ b/hw/core/machine.c
> >>> @@ -28,6 +28,8 @@
> >>>  #include "hw/mem/nvdimm.h"
> >>>  #include "migration/vmstate.h"
> >>>  #include "exec/securable-guest-memory.h"
> >>> +#include "hw/virtio/virtio.h"
> >>> +#include "hw/virtio/virtio-pci.h"
> >>>  
> >>>  GlobalProperty hw_compat_5_1[] = {
> >>>      { "vhost-scsi", "num_queues", "1"},
> >>> @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
> >>>           * areas.
> >>>           */
> >>>          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
> >>> +
> >>> +        /*
> >>> +         * Virtio devices can't count on directly accessing guest
> >>> +         * memory, so they need iommu_platform=on to use normal DMA
> >>> +         * mechanisms.  That requires also disabling legacy virtio
> >>> +         * support for those virtio pci devices which allow it.
> >>> +         */
> >>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> >>> +                                   "on", true);
> >>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> >>> +                                   "on", false);  
> >>
> >> I have not followed all the history (sorry). Should we also set iommu_platform
> >> for virtio-ccw? Halil?
> >>
> > 
> > That line should add iommu_platform for all virtio devices, shouldn't
> > it?
> 
> Yes, sorry. Was misreading that with the line above. 
> 

I believe this is the best we can get. In a sense it is still a
pessimization, but it is a big usability improvement compared to having
to set iommu_platform manually. 

Regards,
Halil
Cornelia Huck Dec. 4, 2020, 5:04 p.m. UTC | #5
On Fri,  4 Dec 2020 16:44:14 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> The default behaviour for virtio devices is not to use the platforms normal
> DMA paths, but instead to use the fact that it's running in a hypervisor
> to directly access guest memory.  That doesn't work if the guest's memory
> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> 
> So, if a securable guest memory mechanism is enabled, then apply the
> iommu_platform=on option so it will go through normal DMA mechanisms.
> Those will presumably have some way of marking memory as shared with
> the hypervisor or hardware so that DMA will work.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/core/machine.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
David Gibson Dec. 8, 2020, 1:54 a.m. UTC | #6
On Fri, Dec 04, 2020 at 03:43:10PM +0100, Halil Pasic wrote:
> On Fri, 4 Dec 2020 09:29:59 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
> > On 04.12.20 09:17, Cornelia Huck wrote:
> > > On Fri, 4 Dec 2020 09:10:36 +0100
> > > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> > > 
> > >> On 04.12.20 06:44, David Gibson wrote:
> > >>> The default behaviour for virtio devices is not to use the platforms normal
> > >>> DMA paths, but instead to use the fact that it's running in a hypervisor
> > >>> to directly access guest memory.  That doesn't work if the guest's memory
> > >>> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
> > >>>
> > >>> So, if a securable guest memory mechanism is enabled, then apply the
> > >>> iommu_platform=on option so it will go through normal DMA mechanisms.
> > >>> Those will presumably have some way of marking memory as shared with
> > >>> the hypervisor or hardware so that DMA will work.
> > >>>
> > >>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > >>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > >>> ---
> > >>>  hw/core/machine.c | 13 +++++++++++++
> > >>>  1 file changed, 13 insertions(+)
> > >>>
> > >>> diff --git a/hw/core/machine.c b/hw/core/machine.c
> > >>> index a67a27d03c..d16273d75d 100644
> > >>> --- a/hw/core/machine.c
> > >>> +++ b/hw/core/machine.c
> > >>> @@ -28,6 +28,8 @@
> > >>>  #include "hw/mem/nvdimm.h"
> > >>>  #include "migration/vmstate.h"
> > >>>  #include "exec/securable-guest-memory.h"
> > >>> +#include "hw/virtio/virtio.h"
> > >>> +#include "hw/virtio/virtio-pci.h"
> > >>>  
> > >>>  GlobalProperty hw_compat_5_1[] = {
> > >>>      { "vhost-scsi", "num_queues", "1"},
> > >>> @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
> > >>>           * areas.
> > >>>           */
> > >>>          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
> > >>> +
> > >>> +        /*
> > >>> +         * Virtio devices can't count on directly accessing guest
> > >>> +         * memory, so they need iommu_platform=on to use normal DMA
> > >>> +         * mechanisms.  That requires also disabling legacy virtio
> > >>> +         * support for those virtio pci devices which allow it.
> > >>> +         */
> > >>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> > >>> +                                   "on", true);
> > >>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> > >>> +                                   "on", false);  
> > >>
> > >> I have not followed all the history (sorry). Should we also set iommu_platform
> > >> for virtio-ccw? Halil?
> > >>
> > > 
> > > That line should add iommu_platform for all virtio devices, shouldn't
> > > it?
> > 
> > Yes, sorry. Was misreading that with the line above. 
> > 
> 
> I believe this is the best we can get. In a sense it is still a
> pessimization,

I'm not really clear on what you're getting at here.

> but it is a big usability improvement compared to having
> to set iommu_platform manually. 
> 
> Regards,
> Halil
>
Christian Borntraeger Dec. 8, 2020, 8:16 a.m. UTC | #7
On 08.12.20 02:54, David Gibson wrote:
> On Fri, Dec 04, 2020 at 03:43:10PM +0100, Halil Pasic wrote:
>> On Fri, 4 Dec 2020 09:29:59 +0100
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 04.12.20 09:17, Cornelia Huck wrote:
>>>> On Fri, 4 Dec 2020 09:10:36 +0100
>>>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>>>
>>>>> On 04.12.20 06:44, David Gibson wrote:
>>>>>> The default behaviour for virtio devices is not to use the platforms normal
>>>>>> DMA paths, but instead to use the fact that it's running in a hypervisor
>>>>>> to directly access guest memory.  That doesn't work if the guest's memory
>>>>>> is protected from hypervisor access, such as with AMD's SEV or POWER's PEF.
>>>>>>
>>>>>> So, if a securable guest memory mechanism is enabled, then apply the
>>>>>> iommu_platform=on option so it will go through normal DMA mechanisms.
>>>>>> Those will presumably have some way of marking memory as shared with
>>>>>> the hypervisor or hardware so that DMA will work.
>>>>>>
>>>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>>> ---
>>>>>>  hw/core/machine.c | 13 +++++++++++++
>>>>>>  1 file changed, 13 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>>>>> index a67a27d03c..d16273d75d 100644
>>>>>> --- a/hw/core/machine.c
>>>>>> +++ b/hw/core/machine.c
>>>>>> @@ -28,6 +28,8 @@
>>>>>>  #include "hw/mem/nvdimm.h"
>>>>>>  #include "migration/vmstate.h"
>>>>>>  #include "exec/securable-guest-memory.h"
>>>>>> +#include "hw/virtio/virtio.h"
>>>>>> +#include "hw/virtio/virtio-pci.h"
>>>>>>  
>>>>>>  GlobalProperty hw_compat_5_1[] = {
>>>>>>      { "vhost-scsi", "num_queues", "1"},
>>>>>> @@ -1169,6 +1171,17 @@ void machine_run_board_init(MachineState *machine)
>>>>>>           * areas.
>>>>>>           */
>>>>>>          machine_set_mem_merge(OBJECT(machine), false, &error_abort);
>>>>>> +
>>>>>> +        /*
>>>>>> +         * Virtio devices can't count on directly accessing guest
>>>>>> +         * memory, so they need iommu_platform=on to use normal DMA
>>>>>> +         * mechanisms.  That requires also disabling legacy virtio
>>>>>> +         * support for those virtio pci devices which allow it.
>>>>>> +         */
>>>>>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
>>>>>> +                                   "on", true);
>>>>>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
>>>>>> +                                   "on", false);  
>>>>>
>>>>> I have not followed all the history (sorry). Should we also set iommu_platform
>>>>> for virtio-ccw? Halil?
>>>>>
>>>>
>>>> That line should add iommu_platform for all virtio devices, shouldn't
>>>> it?
>>>
>>> Yes, sorry. Was misreading that with the line above. 
>>>
>>
>> I believe this is the best we can get. In a sense it is still a
>> pessimization,
> 
> I'm not really clear on what you're getting at here.

I think Halils point is that somebody might come up with a solution where things would
work even without iommu_platform. But as he said, still the best setting we can get
to cover all cases.
Halil Pasic Dec. 8, 2020, 10:28 a.m. UTC | #8
On Tue, 8 Dec 2020 12:54:03 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> > > >>> +         * Virtio devices can't count on directly accessing guest
> > > >>> +         * memory, so they need iommu_platform=on to use normal DMA
> > > >>> +         * mechanisms.  That requires also disabling legacy virtio
> > > >>> +         * support for those virtio pci devices which allow it.
> > > >>> +         */
> > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> > > >>> +                                   "on", true);
> > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> > > >>> +                                   "on", false);    
> > > >>
> > > >> I have not followed all the history (sorry). Should we also set iommu_platform
> > > >> for virtio-ccw? Halil?
> > > >>  
> > > > 
> > > > That line should add iommu_platform for all virtio devices, shouldn't
> > > > it?  
> > > 
> > > Yes, sorry. Was misreading that with the line above. 
> > >   
> > 
> > I believe this is the best we can get. In a sense it is still a
> > pessimization,  
> 
> I'm not really clear on what you're getting at here.

By pessimiziation, I mean that we are going to indicate
_F_PLATFORM_ACCESS even if it isn't necessary, because the guest never
opted in for confidential/memory protection/memory encryption. We have
discussed this before, and I don't see a better solution that works for
everybody.

Regards,
Halil
Cornelia Huck Dec. 8, 2020, 12:50 p.m. UTC | #9
On Tue, 8 Dec 2020 11:28:29 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:

> On Tue, 8 Dec 2020 12:54:03 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > > > >>> +         * Virtio devices can't count on directly accessing guest
> > > > >>> +         * memory, so they need iommu_platform=on to use normal DMA
> > > > >>> +         * mechanisms.  That requires also disabling legacy virtio
> > > > >>> +         * support for those virtio pci devices which allow it.
> > > > >>> +         */
> > > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> > > > >>> +                                   "on", true);
> > > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> > > > >>> +                                   "on", false);      
> > > > >>
> > > > >> I have not followed all the history (sorry). Should we also set iommu_platform
> > > > >> for virtio-ccw? Halil?
> > > > >>    
> > > > > 
> > > > > That line should add iommu_platform for all virtio devices, shouldn't
> > > > > it?    
> > > > 
> > > > Yes, sorry. Was misreading that with the line above. 
> > > >     
> > > 
> > > I believe this is the best we can get. In a sense it is still a
> > > pessimization,    
> > 
> > I'm not really clear on what you're getting at here.  
> 
> By pessimiziation, I mean that we are going to indicate
> _F_PLATFORM_ACCESS even if it isn't necessary, because the guest never
> opted in for confidential/memory protection/memory encryption. We have
> discussed this before, and I don't see a better solution that works for
> everybody.

If you consider specifying the secure guest option as a way to tell
QEMU to make everything ready for running a secure guest, I'd certainly
consider it necessary. If you do not want to force it, you should not
do the secure guest preparation setup.
David Gibson Dec. 17, 2020, 5:53 a.m. UTC | #10
On Tue, Dec 08, 2020 at 01:50:05PM +0100, Cornelia Huck wrote:
> On Tue, 8 Dec 2020 11:28:29 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Tue, 8 Dec 2020 12:54:03 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > > > >>> +         * Virtio devices can't count on directly accessing guest
> > > > > >>> +         * memory, so they need iommu_platform=on to use normal DMA
> > > > > >>> +         * mechanisms.  That requires also disabling legacy virtio
> > > > > >>> +         * support for those virtio pci devices which allow it.
> > > > > >>> +         */
> > > > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
> > > > > >>> +                                   "on", true);
> > > > > >>> +        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
> > > > > >>> +                                   "on", false);      
> > > > > >>
> > > > > >> I have not followed all the history (sorry). Should we also set iommu_platform
> > > > > >> for virtio-ccw? Halil?
> > > > > >>    
> > > > > > 
> > > > > > That line should add iommu_platform for all virtio devices, shouldn't
> > > > > > it?    
> > > > > 
> > > > > Yes, sorry. Was misreading that with the line above. 
> > > > >     
> > > > 
> > > > I believe this is the best we can get. In a sense it is still a
> > > > pessimization,    
> > > 
> > > I'm not really clear on what you're getting at here.  
> > 
> > By pessimiziation, I mean that we are going to indicate
> > _F_PLATFORM_ACCESS even if it isn't necessary, because the guest never
> > opted in for confidential/memory protection/memory encryption. We have
> > discussed this before, and I don't see a better solution that works for
> > everybody.
> 
> If you consider specifying the secure guest option as a way to tell
> QEMU to make everything ready for running a secure guest, I'd certainly
> consider it necessary. If you do not want to force it, you should not
> do the secure guest preparation setup.

Right, that's my feeling as well.

I'm also of the opinion that !F_PLATFORM_ACCESS is kind of a nasty
hack that has some other problems (e.g. it means an L1 can't safely
pass the device into an L2).
diff mbox series

Patch

diff --git a/hw/core/machine.c b/hw/core/machine.c
index a67a27d03c..d16273d75d 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -28,6 +28,8 @@ 
 #include "hw/mem/nvdimm.h"
 #include "migration/vmstate.h"
 #include "exec/securable-guest-memory.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-pci.h"
 
 GlobalProperty hw_compat_5_1[] = {
     { "vhost-scsi", "num_queues", "1"},
@@ -1169,6 +1171,17 @@  void machine_run_board_init(MachineState *machine)
          * areas.
          */
         machine_set_mem_merge(OBJECT(machine), false, &error_abort);
+
+        /*
+         * Virtio devices can't count on directly accessing guest
+         * memory, so they need iommu_platform=on to use normal DMA
+         * mechanisms.  That requires also disabling legacy virtio
+         * support for those virtio pci devices which allow it.
+         */
+        object_register_sugar_prop(TYPE_VIRTIO_PCI, "disable-legacy",
+                                   "on", true);
+        object_register_sugar_prop(TYPE_VIRTIO_DEVICE, "iommu_platform",
+                                   "on", false);
     }
 
     machine_class->init(machine);