All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>,
	qemu-riscv@nongnu.org, "Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Helge Deller" <deller@gmx.de>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, "Greg Kurz" <groug@kaod.org>,
	qemu-arm@nongnu.org, qemu-ppc@nongnu.org,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Artyom Tarasenko" <atar4qemu@gmail.com>
Subject: Re: [PATCH 5/7] hw: Have machines Kconfig-select FW_CFG
Date: Tue, 27 Apr 2021 11:56:21 +1000	[thread overview]
Message-ID: <YIdvRYFr2cViBM9+@yekko.fritz.box> (raw)
In-Reply-To: <aec5eea-8beb-a59-fda1-4c1ba4374c5@eik.bme.hu>

[-- Attachment #1: Type: text/plain, Size: 4189 bytes --]

On Tue, Apr 27, 2021 at 12:03:42AM +0200, BALATON Zoltan wrote:
> On Mon, 26 Apr 2021, Philippe Mathieu-Daudé wrote:
> > Beside the loongson3-virt machine (MIPS), the following machines
> > also use the fw_cfg device:
> > 
> > - ARM: virt & sbsa-ref
> > - HPPA: generic machine
> > - X86: ACPI based (pc & microvm)
> > - PPC64: various
> > - SPARC: sun4m & sun4u
> > 
> > Add their FW_CFG Kconfig dependency.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > hw/arm/Kconfig     | 2 ++
> > hw/hppa/Kconfig    | 1 +
> > hw/i386/Kconfig    | 2 ++
> > hw/ppc/Kconfig     | 1 +
> > hw/sparc/Kconfig   | 1 +
> > hw/sparc64/Kconfig | 1 +
> > 6 files changed, 8 insertions(+)
> > 
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 8c37cf00da7..3b2641e39dc 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -8,6 +8,7 @@ config ARM_VIRT
> >     imply TPM_TIS_SYSBUS
> >     select ARM_GIC
> >     select ACPI
> > +    select FW_CFG
> >     select ARM_SMMUV3
> >     select GPIO_KEY
> >     select FW_CFG_DMA
> > @@ -216,6 +217,7 @@ config SBSA_REF
> >     select PL061 # GPIO
> >     select USB_EHCI_SYSBUS
> >     select WDT_SBSA
> > +    select FW_CFG
> > 
> > config SABRELITE
> >     bool
> > diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> > index 22948db0256..45f40e09224 100644
> > --- a/hw/hppa/Kconfig
> > +++ b/hw/hppa/Kconfig
> > @@ -14,3 +14,4 @@ config DINO
> >     select LASIPS2
> >     select PARALLEL
> >     select ARTIST
> > +    select FW_CFG
> > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> > index 7f91f30877f..9e4039a2dce 100644
> > --- a/hw/i386/Kconfig
> > +++ b/hw/i386/Kconfig
> > @@ -52,6 +52,7 @@ config PC_ACPI
> >     select SMBUS_EEPROM
> >     select PFLASH_CFI01
> >     depends on ACPI_SMBUS
> > +    select FW_CFG
> > 
> > config I440FX
> >     bool
> > @@ -106,6 +107,7 @@ config MICROVM
> >     select ACPI_HW_REDUCED
> >     select PCI_EXPRESS_GENERIC_BRIDGE
> >     select USB_XHCI_SYSBUS
> > +    select FW_CFG
> > 
> > config X86_IOMMU
> >     bool
> > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> > index d11dc30509d..a7ba8283bf1 100644
> > --- a/hw/ppc/Kconfig
> > +++ b/hw/ppc/Kconfig
> > @@ -131,6 +131,7 @@ config VIRTEX
> > # Only used by 64-bit targets
> > config FW_CFG_PPC
> >     bool
> > +    select FW_CFG
> 
> Why do we need a separate config option here if all it does is select FW_CFG
> and also in meson.build it only seems to add fw_cfg.c? (Unlike FW_CFG_DMA
> which seems to add other files so another option makes sense for that case).
> Could we just use FW_CFG directly and drop the PPC specific option like you
> did for MIPS?
> 
> Also the comment saying "Only used by 64-bit targets" seems to be wrong as
> it is also selected by MAC_OLDWORLD that's definitely a 32-bit machine (and
> MAC_NEWWORLD that can be both 32 or 64 bit) so maybe this option used to do
> something previously but now seems to be equivalent to just FW_CFG. So could
> it be dropped and use FW_CFG instead to simplify this or what's the reason
> to keep a PPC specific option for it?

Actually... good point.  I don't see any reason for this config option either.

> 
> Regards,
> BALATON Zoltan
> 
> > 
> > config FDT_PPC
> >     bool
> > diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
> > index 8dcb10086fd..267bf45fa21 100644
> > --- a/hw/sparc/Kconfig
> > +++ b/hw/sparc/Kconfig
> > @@ -15,6 +15,7 @@ config SUN4M
> >     select STP2000
> >     select CHRP_NVRAM
> >     select OR_IRQ
> > +    select FW_CFG
> > 
> > config LEON3
> >     bool
> > diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
> > index 980a201bb73..c17b34b9d5b 100644
> > --- a/hw/sparc64/Kconfig
> > +++ b/hw/sparc64/Kconfig
> > @@ -13,6 +13,7 @@ config SUN4U
> >     select PCKBD
> >     select SIMBA
> >     select CHRP_NVRAM
> > +    select FW_CFG
> > 
> > config NIAGARA
> >     bool
> > 


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: David Gibson <david@gibson.dropbear.id.au>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
	"Thomas Huth" <thuth@redhat.com>,
	qemu-riscv@nongnu.org, "Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Helge Deller" <deller@gmx.de>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Greg Kurz" <groug@kaod.org>,
	qemu-arm@nongnu.org, qemu-ppc@nongnu.org,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Laszlo Ersek" <lersek@redhat.com>,
	"Artyom Tarasenko" <atar4qemu@gmail.com>
Subject: Re: [PATCH 5/7] hw: Have machines Kconfig-select FW_CFG
Date: Tue, 27 Apr 2021 11:56:21 +1000	[thread overview]
Message-ID: <YIdvRYFr2cViBM9+@yekko.fritz.box> (raw)
In-Reply-To: <aec5eea-8beb-a59-fda1-4c1ba4374c5@eik.bme.hu>

[-- Attachment #1: Type: text/plain, Size: 4189 bytes --]

On Tue, Apr 27, 2021 at 12:03:42AM +0200, BALATON Zoltan wrote:
> On Mon, 26 Apr 2021, Philippe Mathieu-Daudé wrote:
> > Beside the loongson3-virt machine (MIPS), the following machines
> > also use the fw_cfg device:
> > 
> > - ARM: virt & sbsa-ref
> > - HPPA: generic machine
> > - X86: ACPI based (pc & microvm)
> > - PPC64: various
> > - SPARC: sun4m & sun4u
> > 
> > Add their FW_CFG Kconfig dependency.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> > hw/arm/Kconfig     | 2 ++
> > hw/hppa/Kconfig    | 1 +
> > hw/i386/Kconfig    | 2 ++
> > hw/ppc/Kconfig     | 1 +
> > hw/sparc/Kconfig   | 1 +
> > hw/sparc64/Kconfig | 1 +
> > 6 files changed, 8 insertions(+)
> > 
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 8c37cf00da7..3b2641e39dc 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -8,6 +8,7 @@ config ARM_VIRT
> >     imply TPM_TIS_SYSBUS
> >     select ARM_GIC
> >     select ACPI
> > +    select FW_CFG
> >     select ARM_SMMUV3
> >     select GPIO_KEY
> >     select FW_CFG_DMA
> > @@ -216,6 +217,7 @@ config SBSA_REF
> >     select PL061 # GPIO
> >     select USB_EHCI_SYSBUS
> >     select WDT_SBSA
> > +    select FW_CFG
> > 
> > config SABRELITE
> >     bool
> > diff --git a/hw/hppa/Kconfig b/hw/hppa/Kconfig
> > index 22948db0256..45f40e09224 100644
> > --- a/hw/hppa/Kconfig
> > +++ b/hw/hppa/Kconfig
> > @@ -14,3 +14,4 @@ config DINO
> >     select LASIPS2
> >     select PARALLEL
> >     select ARTIST
> > +    select FW_CFG
> > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> > index 7f91f30877f..9e4039a2dce 100644
> > --- a/hw/i386/Kconfig
> > +++ b/hw/i386/Kconfig
> > @@ -52,6 +52,7 @@ config PC_ACPI
> >     select SMBUS_EEPROM
> >     select PFLASH_CFI01
> >     depends on ACPI_SMBUS
> > +    select FW_CFG
> > 
> > config I440FX
> >     bool
> > @@ -106,6 +107,7 @@ config MICROVM
> >     select ACPI_HW_REDUCED
> >     select PCI_EXPRESS_GENERIC_BRIDGE
> >     select USB_XHCI_SYSBUS
> > +    select FW_CFG
> > 
> > config X86_IOMMU
> >     bool
> > diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> > index d11dc30509d..a7ba8283bf1 100644
> > --- a/hw/ppc/Kconfig
> > +++ b/hw/ppc/Kconfig
> > @@ -131,6 +131,7 @@ config VIRTEX
> > # Only used by 64-bit targets
> > config FW_CFG_PPC
> >     bool
> > +    select FW_CFG
> 
> Why do we need a separate config option here if all it does is select FW_CFG
> and also in meson.build it only seems to add fw_cfg.c? (Unlike FW_CFG_DMA
> which seems to add other files so another option makes sense for that case).
> Could we just use FW_CFG directly and drop the PPC specific option like you
> did for MIPS?
> 
> Also the comment saying "Only used by 64-bit targets" seems to be wrong as
> it is also selected by MAC_OLDWORLD that's definitely a 32-bit machine (and
> MAC_NEWWORLD that can be both 32 or 64 bit) so maybe this option used to do
> something previously but now seems to be equivalent to just FW_CFG. So could
> it be dropped and use FW_CFG instead to simplify this or what's the reason
> to keep a PPC specific option for it?

Actually... good point.  I don't see any reason for this config option either.

> 
> Regards,
> BALATON Zoltan
> 
> > 
> > config FDT_PPC
> >     bool
> > diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
> > index 8dcb10086fd..267bf45fa21 100644
> > --- a/hw/sparc/Kconfig
> > +++ b/hw/sparc/Kconfig
> > @@ -15,6 +15,7 @@ config SUN4M
> >     select STP2000
> >     select CHRP_NVRAM
> >     select OR_IRQ
> > +    select FW_CFG
> > 
> > config LEON3
> >     bool
> > diff --git a/hw/sparc64/Kconfig b/hw/sparc64/Kconfig
> > index 980a201bb73..c17b34b9d5b 100644
> > --- a/hw/sparc64/Kconfig
> > +++ b/hw/sparc64/Kconfig
> > @@ -13,6 +13,7 @@ config SUN4U
> >     select PCKBD
> >     select SIMBA
> >     select CHRP_NVRAM
> > +    select FW_CFG
> > 
> > config NIAGARA
> >     bool
> > 


-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2021-04-27  2:03 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 19:35 [PATCH 0/7] hw/nvram/fw_cfg: Do not build device if not needed (Spring cleanup) Philippe Mathieu-Daudé
2021-04-26 19:35 ` Philippe Mathieu-Daudé
2021-04-26 19:35 ` [PATCH 1/7] stubs: Restrict fw_cfg stubs to sysemu Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-28 16:23   ` Laszlo Ersek
2021-04-26 19:35 ` [PATCH 2/7] hw/nvram: Rename FW_CFG_MIPS as generic FW_CFG Kconfig symbol Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-26 19:35 ` [PATCH 3/7] hw/nvram: Declare FW_CFG_DMA Kconfig symbol in hw/nvram/ Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-26 19:35 ` [PATCH 4/7] hw/acpi/vmgenid: Make ACPI_VMGENID depends on FW_CFG Kconfig Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-28 16:25   ` Laszlo Ersek
2021-04-26 19:35 ` [PATCH 5/7] hw: Have machines Kconfig-select FW_CFG Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-26 22:03   ` BALATON Zoltan
2021-04-26 22:03     ` BALATON Zoltan
2021-04-27  1:56     ` David Gibson [this message]
2021-04-27  1:56       ` David Gibson
2021-04-27  1:54   ` David Gibson
2021-04-27  1:54     ` David Gibson
2021-04-28 16:33   ` Laszlo Ersek
2021-04-28 16:33     ` Laszlo Ersek
2021-04-28 18:50   ` Eduardo Habkost
2021-04-28 18:50     ` Eduardo Habkost
2021-04-26 19:35 ` [PATCH 6/7] hw/{arm,hppa,riscv}: Add fw_cfg arch-specific stub Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-28 16:44   ` Laszlo Ersek
2021-04-28 16:44     ` Laszlo Ersek
2021-04-28 17:23     ` Philippe Mathieu-Daudé
2021-04-28 17:23       ` Philippe Mathieu-Daudé
2021-04-26 19:35 ` [PATCH 7/7] hw/nvram: Do not build FW_CFG if not required Philippe Mathieu-Daudé
2021-04-26 19:35   ` Philippe Mathieu-Daudé
2021-04-28 17:29   ` Philippe Mathieu-Daudé
2021-04-28 17:29     ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YIdvRYFr2cViBM9+@yekko.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=atar4qemu@gmail.com \
    --cc=balaton@eik.bme.hu \
    --cc=deller@gmx.de \
    --cc=ehabkost@redhat.com \
    --cc=groug@kaod.org \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=qemu-riscv@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.