* [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* @ 2020-04-09 12:10 Leif Lindholm 2020-04-09 13:07 ` Ard Biesheuvel 2020-04-09 13:56 ` Laszlo Ersek 0 siblings, 2 replies; 7+ messages in thread From: Leif Lindholm @ 2020-04-09 12:10 UTC (permalink / raw) To: devel; +Cc: Ard Biesheuvel, Laszlo Ersek, Leendert van Doorn From: Leendert van Doorn <leendert@microsoft.com> Enable conditional support for NVMe storage in ArmVirtQemu/ QemVirtQemuKernel in order to simplify booting/installing operating systems that don't support virtio. [Conditionalised driver inclusion] Signed-off-by: Leif Lindholm <leif@nuviainc.com> --- ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ 3 files changed, 23 insertions(+) diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc index 8c77fc46427b..6f93032ac064 100644 --- a/ArmVirtPkg/ArmVirtQemu.dsc +++ b/ArmVirtPkg/ArmVirtQemu.dsc @@ -28,6 +28,7 @@ [Defines] # -D FLAG=VALUE # DEFINE TTY_TERMINAL = FALSE + DEFINE NVME_ENABLE = FALSE DEFINE SECURE_BOOT_ENABLE = FALSE DEFINE TPM2_ENABLE = FALSE DEFINE TPM2_CONFIG_ENABLE = FALSE @@ -447,6 +448,13 @@ [Components.common] MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf + # + # NVME Driver + # +!if $(NVME_ENABLE) == TRUE + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf +!endif + # # SMBIOS Support # diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc index aaba0b1c8840..45f0dd65be33 100644 --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc @@ -128,6 +128,13 @@ [FV.FvMain] INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf + # + # NVME Driver + # +!if $(NVME_ENABLE) == TRUE + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf +!endif + # # SMBIOS Support # diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc index 4d82a77213ec..5dd4b1cf29f4 100644 --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc @@ -28,6 +28,7 @@ [Defines] # -D FLAG=VALUE # DEFINE TTY_TERMINAL = FALSE + DEFINE NVME_ENABLE = FALSE DEFINE SECURE_BOOT_ENABLE = FALSE # @@ -382,6 +383,13 @@ [Components.common] MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf + # + # NVME Driver + # +!if $(NVME_ENABLE) == TRUE + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf +!endif + # # SMBIOS Support # -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 12:10 [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* Leif Lindholm @ 2020-04-09 13:07 ` Ard Biesheuvel 2020-04-09 14:31 ` Leif Lindholm 2020-04-09 13:56 ` Laszlo Ersek 1 sibling, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2020-04-09 13:07 UTC (permalink / raw) To: Leif Lindholm, devel; +Cc: Laszlo Ersek, Leendert van Doorn On 4/9/20 2:10 PM, Leif Lindholm wrote: > From: Leendert van Doorn <leendert@microsoft.com> > > Enable conditional support for NVMe storage in ArmVirtQemu/ > QemVirtQemuKernel in order to simplify booting/installing operating > systems that don't support virtio. > > [Conditionalised driver inclusion] Why? > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > --- > ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ > ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ > 3 files changed, 23 insertions(+) > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index 8c77fc46427b..6f93032ac064 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -28,6 +28,7 @@ [Defines] > # -D FLAG=VALUE > # > DEFINE TTY_TERMINAL = FALSE > + DEFINE NVME_ENABLE = FALSE > DEFINE SECURE_BOOT_ENABLE = FALSE > DEFINE TPM2_ENABLE = FALSE > DEFINE TPM2_CONFIG_ENABLE = FALSE > @@ -447,6 +448,13 @@ [Components.common] > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > index aaba0b1c8840..45f0dd65be33 100644 > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > @@ -128,6 +128,13 @@ [FV.FvMain] > INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 4d82a77213ec..5dd4b1cf29f4 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -28,6 +28,7 @@ [Defines] > # -D FLAG=VALUE > # > DEFINE TTY_TERMINAL = FALSE > + DEFINE NVME_ENABLE = FALSE > DEFINE SECURE_BOOT_ENABLE = FALSE > > # > @@ -382,6 +383,13 @@ [Components.common] > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 13:07 ` Ard Biesheuvel @ 2020-04-09 14:31 ` Leif Lindholm 2020-04-09 14:36 ` Ard Biesheuvel 0 siblings, 1 reply; 7+ messages in thread From: Leif Lindholm @ 2020-04-09 14:31 UTC (permalink / raw) To: Ard Biesheuvel; +Cc: devel, Laszlo Ersek, Leendert van Doorn On Thu, Apr 09, 2020 at 15:07:30 +0200, Ard Biesheuvel wrote: > On 4/9/20 2:10 PM, Leif Lindholm wrote: > > From: Leendert van Doorn <leendert@microsoft.com> > > > > Enable conditional support for NVMe storage in ArmVirtQemu/ > > QemVirtQemuKernel in order to simplify booting/installing operating > > systems that don't support virtio. > > > > [Conditionalised driver inclusion] > > Why? Well, it adds size and complexity for a target most frequently used to model a pure virtual environment? Size difference isn't huge though: FVMAIN [99%Full] 5337920 total, 5337864 used, 56 free FVMAIN_COMPACT [39%Full] 2093056 total, 829392 used, 1263664 free vs. FVMAIN [99%Full] 5300992 total, 5300936 used, 56 free FVMAIN_COMPACT [38%Full] 2093056 total, 804128 used, 1288928 free If that's not an issue, I'm happy to drop it and my bracketing. / Leif > > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > > --- > > ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ > > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ > > 3 files changed, 23 insertions(+) > > > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > > index 8c77fc46427b..6f93032ac064 100644 > > --- a/ArmVirtPkg/ArmVirtQemu.dsc > > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > > @@ -28,6 +28,7 @@ [Defines] > > # -D FLAG=VALUE > > # > > DEFINE TTY_TERMINAL = FALSE > > + DEFINE NVME_ENABLE = FALSE > > DEFINE SECURE_BOOT_ENABLE = FALSE > > DEFINE TPM2_ENABLE = FALSE > > DEFINE TPM2_CONFIG_ENABLE = FALSE > > @@ -447,6 +448,13 @@ [Components.common] > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > index aaba0b1c8840..45f0dd65be33 100644 > > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > @@ -128,6 +128,13 @@ [FV.FvMain] > > INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > index 4d82a77213ec..5dd4b1cf29f4 100644 > > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > @@ -28,6 +28,7 @@ [Defines] > > # -D FLAG=VALUE > > # > > DEFINE TTY_TERMINAL = FALSE > > + DEFINE NVME_ENABLE = FALSE > > DEFINE SECURE_BOOT_ENABLE = FALSE > > # > > @@ -382,6 +383,13 @@ [Components.common] > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 14:31 ` Leif Lindholm @ 2020-04-09 14:36 ` Ard Biesheuvel 0 siblings, 0 replies; 7+ messages in thread From: Ard Biesheuvel @ 2020-04-09 14:36 UTC (permalink / raw) To: Leif Lindholm; +Cc: devel, Laszlo Ersek, Leendert van Doorn On 4/9/20 4:31 PM, Leif Lindholm wrote: > On Thu, Apr 09, 2020 at 15:07:30 +0200, Ard Biesheuvel wrote: >> On 4/9/20 2:10 PM, Leif Lindholm wrote: >>> From: Leendert van Doorn <leendert@microsoft.com> >>> >>> Enable conditional support for NVMe storage in ArmVirtQemu/ >>> QemVirtQemuKernel in order to simplify booting/installing operating >>> systems that don't support virtio. >>> >>> [Conditionalised driver inclusion] >> >> Why? > > Well, it adds size and complexity for a target most frequently used to > model a pure virtual environment? > > Size difference isn't huge though: > FVMAIN [99%Full] 5337920 total, 5337864 used, 56 free > FVMAIN_COMPACT [39%Full] 2093056 total, 829392 used, 1263664 free > vs. > FVMAIN [99%Full] 5300992 total, 5300936 used, 56 free > FVMAIN_COMPACT [38%Full] 2093056 total, 804128 used, 1288928 free > > If that's not an issue, I'm happy to drop it and my bracketing. > I agree that in general, driver stacks like network or TPM should be opt-in, but given the reason you are adding this [compatibility with other OSes], I'd prefer to have this one always built in. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 12:10 [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* Leif Lindholm 2020-04-09 13:07 ` Ard Biesheuvel @ 2020-04-09 13:56 ` Laszlo Ersek 2020-04-09 14:53 ` Leif Lindholm 1 sibling, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2020-04-09 13:56 UTC (permalink / raw) To: Leif Lindholm, devel; +Cc: Ard Biesheuvel, Leendert van Doorn On 04/09/20 14:10, Leif Lindholm wrote: > From: Leendert van Doorn <leendert@microsoft.com> > > Enable conditional support for NVMe storage in ArmVirtQemu/ > QemVirtQemuKernel in order to simplify booting/installing operating > systems that don't support virtio. (1) We also have UsbMassStorageDxe in ArmVirtQemu*, which can drive QEMU's "usb-storage" device model. In case that device+driver combo has been tested too, and unsuccessfully, then I suggest explicitly stating in the commit message: "don't support virtio or usb-storage". If UsbMassStorageDxe has *not* been tested, then I agree it should not be mentioned here at all. For more details -- in case someone is interested in testing "usb-storage" --, please refer to commit f9c59fa44ae2 ("OvmfPkg/QemuBootOrderLib: recognize "usb-storage" devices in XHCI ports", 2017-09-22). That commit message includes both QEMU command line and libvirt domain XML examples. > > [Conditionalised driver inclusion] > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > --- > ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ > ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ > 3 files changed, 23 insertions(+) (2) I realize I'm being arbitrary, with regard to what driver should be conditional and what should always be there. But in OvmfPkg, NvmExpressDxe is included unconditionally, and I think that should work here too. Of course, if you *want* to make NvmExpressDxe excludable from the ArmVirtQemu* platforms, then I'm fine with that. So, I'm proposing (1) and (2) only as food for thought; dependent on your stance on them, I'm OK with this patch: Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks, Laszlo > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index 8c77fc46427b..6f93032ac064 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -28,6 +28,7 @@ [Defines] > # -D FLAG=VALUE > # > DEFINE TTY_TERMINAL = FALSE > + DEFINE NVME_ENABLE = FALSE > DEFINE SECURE_BOOT_ENABLE = FALSE > DEFINE TPM2_ENABLE = FALSE > DEFINE TPM2_CONFIG_ENABLE = FALSE > @@ -447,6 +448,13 @@ [Components.common] > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > index aaba0b1c8840..45f0dd65be33 100644 > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > @@ -128,6 +128,13 @@ [FV.FvMain] > INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 4d82a77213ec..5dd4b1cf29f4 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -28,6 +28,7 @@ [Defines] > # -D FLAG=VALUE > # > DEFINE TTY_TERMINAL = FALSE > + DEFINE NVME_ENABLE = FALSE > DEFINE SECURE_BOOT_ENABLE = FALSE > > # > @@ -382,6 +383,13 @@ [Components.common] > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > + # > + # NVME Driver > + # > +!if $(NVME_ENABLE) == TRUE > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > +!endif > + > # > # SMBIOS Support > # > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 13:56 ` Laszlo Ersek @ 2020-04-09 14:53 ` Leif Lindholm 2020-04-09 17:43 ` Leif Lindholm 0 siblings, 1 reply; 7+ messages in thread From: Leif Lindholm @ 2020-04-09 14:53 UTC (permalink / raw) To: Laszlo Ersek; +Cc: devel, Ard Biesheuvel, Leendert van Doorn On Thu, Apr 09, 2020 at 15:56:20 +0200, Laszlo Ersek wrote: > On 04/09/20 14:10, Leif Lindholm wrote: > > From: Leendert van Doorn <leendert@microsoft.com> > > > > Enable conditional support for NVMe storage in ArmVirtQemu/ > > QemVirtQemuKernel in order to simplify booting/installing operating > > systems that don't support virtio. > > (1) We also have UsbMassStorageDxe in ArmVirtQemu*, which can drive > QEMU's "usb-storage" device model. > > In case that device+driver combo has been tested too, and > unsuccessfully, then I suggest explicitly stating in the commit message: > "don't support virtio or usb-storage". Well, in this particular case we were more interested in the (much) lower protocol overhead. But that is a very good point in general. > If UsbMassStorageDxe has *not* been tested, then I agree it should not > be mentioned here at all. I have not, but I'll chuck it on the pile of stuff to test at some point. > For more details -- in case someone is interested in testing > "usb-storage" --, please refer to commit f9c59fa44ae2 > ("OvmfPkg/QemuBootOrderLib: recognize "usb-storage" devices in XHCI > ports", 2017-09-22). That commit message includes both QEMU command line > and libvirt domain XML examples. Thanks! > > [Conditionalised driver inclusion] > > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > > --- > > ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ > > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ > > 3 files changed, 23 insertions(+) > > (2) I realize I'm being arbitrary, with regard to what driver should be > conditional and what should always be there. But in OvmfPkg, > NvmExpressDxe is included unconditionally, and I think that should work > here too. > > Of course, if you *want* to make NvmExpressDxe excludable from the > ArmVirtQemu* platforms, then I'm fine with that. I don't. I just succumbed to premature optimization. Do you want to see a v2, or do I drop the bits before submission? Regards, Leif > So, I'm proposing (1) and (2) only as food for thought; dependent on > your stance on them, I'm OK with this patch: > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> > > Thanks, > Laszlo > > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > > index 8c77fc46427b..6f93032ac064 100644 > > --- a/ArmVirtPkg/ArmVirtQemu.dsc > > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > > @@ -28,6 +28,7 @@ [Defines] > > # -D FLAG=VALUE > > # > > DEFINE TTY_TERMINAL = FALSE > > + DEFINE NVME_ENABLE = FALSE > > DEFINE SECURE_BOOT_ENABLE = FALSE > > DEFINE TPM2_ENABLE = FALSE > > DEFINE TPM2_CONFIG_ENABLE = FALSE > > @@ -447,6 +448,13 @@ [Components.common] > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > index aaba0b1c8840..45f0dd65be33 100644 > > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > @@ -128,6 +128,13 @@ [FV.FvMain] > > INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > index 4d82a77213ec..5dd4b1cf29f4 100644 > > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > @@ -28,6 +28,7 @@ [Defines] > > # -D FLAG=VALUE > > # > > DEFINE TTY_TERMINAL = FALSE > > + DEFINE NVME_ENABLE = FALSE > > DEFINE SECURE_BOOT_ENABLE = FALSE > > > > # > > @@ -382,6 +383,13 @@ [Components.common] > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > + # > > + # NVME Driver > > + # > > +!if $(NVME_ENABLE) == TRUE > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > +!endif > > + > > # > > # SMBIOS Support > > # > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* 2020-04-09 14:53 ` Leif Lindholm @ 2020-04-09 17:43 ` Leif Lindholm 0 siblings, 0 replies; 7+ messages in thread From: Leif Lindholm @ 2020-04-09 17:43 UTC (permalink / raw) To: Laszlo Ersek; +Cc: devel, Ard Biesheuvel, Leendert van Doorn On Thu, Apr 09, 2020 at 15:53:59 +0100, Leif Lindholm wrote: > > > [Conditionalised driver inclusion] > > > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > > > --- > > > ArmVirtPkg/ArmVirtQemu.dsc | 8 ++++++++ > > > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 +++++++ > > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 8 ++++++++ > > > 3 files changed, 23 insertions(+) > > > > (2) I realize I'm being arbitrary, with regard to what driver should be > > conditional and what should always be there. But in OvmfPkg, > > NvmExpressDxe is included unconditionally, and I think that should work > > here too. > > > > Of course, if you *want* to make NvmExpressDxe excludable from the > > ArmVirtQemu* platforms, then I'm fine with that. > > I don't. I just succumbed to premature optimization. > Do you want to see a v2, or do I drop the bits before submission? > > Regards, > > Leif > > > So, I'm proposing (1) and (2) only as food for thought; dependent on > > your stance on them, I'm OK with this patch: > > > > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Given Ard's stated opinion in other email, I'm taking this and dropping the conditionals. Pushed as a5d8a399635d. Thanks! > > > > Thanks, > > Laszlo > > > > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > > > index 8c77fc46427b..6f93032ac064 100644 > > > --- a/ArmVirtPkg/ArmVirtQemu.dsc > > > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > > > @@ -28,6 +28,7 @@ [Defines] > > > # -D FLAG=VALUE > > > # > > > DEFINE TTY_TERMINAL = FALSE > > > + DEFINE NVME_ENABLE = FALSE > > > DEFINE SECURE_BOOT_ENABLE = FALSE > > > DEFINE TPM2_ENABLE = FALSE > > > DEFINE TPM2_CONFIG_ENABLE = FALSE > > > @@ -447,6 +448,13 @@ [Components.common] > > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > > > + # > > > + # NVME Driver > > > + # > > > +!if $(NVME_ENABLE) == TRUE > > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > > +!endif > > > + > > > # > > > # SMBIOS Support > > > # > > > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > > index aaba0b1c8840..45f0dd65be33 100644 > > > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > > > @@ -128,6 +128,13 @@ [FV.FvMain] > > > INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > > INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > > > + # > > > + # NVME Driver > > > + # > > > +!if $(NVME_ENABLE) == TRUE > > > + INF MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > > +!endif > > > + > > > # > > > # SMBIOS Support > > > # > > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > > index 4d82a77213ec..5dd4b1cf29f4 100644 > > > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > > > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > > > @@ -28,6 +28,7 @@ [Defines] > > > # -D FLAG=VALUE > > > # > > > DEFINE TTY_TERMINAL = FALSE > > > + DEFINE NVME_ENABLE = FALSE > > > DEFINE SECURE_BOOT_ENABLE = FALSE > > > > > > # > > > @@ -382,6 +383,13 @@ [Components.common] > > > MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf > > > MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf > > > > > > + # > > > + # NVME Driver > > > + # > > > +!if $(NVME_ENABLE) == TRUE > > > + MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressDxe.inf > > > +!endif > > > + > > > # > > > # SMBIOS Support > > > # > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-09 17:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-09 12:10 [PATCH 1/1] ArmVirtPkg: Include NVMe support in ArmVirtQemu* Leif Lindholm 2020-04-09 13:07 ` Ard Biesheuvel 2020-04-09 14:31 ` Leif Lindholm 2020-04-09 14:36 ` Ard Biesheuvel 2020-04-09 13:56 ` Laszlo Ersek 2020-04-09 14:53 ` Leif Lindholm 2020-04-09 17:43 ` Leif Lindholm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox