Message ID | 20201204054415.579042-13-david@gibson.dropbear.id.au (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Generalize memory encryption models | expand |
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?
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?
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.
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
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>
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 >
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.
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
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.
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 --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);