diff mbox series

[v3,17/19] hw/arm: Automatically select the 'virt' machine on KVM

Message ID 20200316160634.3386-18-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series Support disabling TCG on ARM (part 2) | expand

Commit Message

Philippe Mathieu-Daudé March 16, 2020, 4:06 p.m. UTC 
When building a KVM-only QEMU, the 'virt' machine is a good
default :)

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Richard Henderson March 16, 2020, 8:06 p.m. UTC | #1 
On 3/16/20 9:06 AM, Philippe Mathieu-Daudé wrote:
> When building a KVM-only QEMU, the 'virt' machine is a good
> default :)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index d0903d8544..8e801cd15f 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM_VIRT
>      bool
> +    default y if KVM
>      imply PCI_DEVICES
>      imply TEST_DEVICES
>      imply VFIO_AMD_XGBE
> 

Likewise SBSA_REF?  Otherwise, what is this for?
Did you remove ARM_VIRT from default-config/*?


r~
Philippe Mathieu-Daudé Sept. 29, 2020, 6:26 p.m. UTC | #2 
On 3/16/20 9:06 PM, Richard Henderson wrote:
> On 3/16/20 9:06 AM, Philippe Mathieu-Daudé wrote:
>> When building a KVM-only QEMU, the 'virt' machine is a good
>> default :)
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/arm/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>> index d0903d8544..8e801cd15f 100644
>> --- a/hw/arm/Kconfig
>> +++ b/hw/arm/Kconfig
>> @@ -1,5 +1,6 @@
>>  config ARM_VIRT
>>      bool
>> +    default y if KVM
>>      imply PCI_DEVICES
>>      imply TEST_DEVICES
>>      imply VFIO_AMD_XGBE
>>
> 
> Likewise SBSA_REF?

OK.

> Otherwise, what is this for?
> Did you remove ARM_VIRT from default-config/*?

This is to use custom config (and easily test by
blowing default-config/).

> 
> 
> r~
>
Peter Maydell Sept. 29, 2020, 8:06 p.m. UTC | #3 
On Mon, 16 Mar 2020 at 16:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> When building a KVM-only QEMU, the 'virt' machine is a good
> default :)
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index d0903d8544..8e801cd15f 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM_VIRT
>      bool
> +    default y if KVM
>      imply PCI_DEVICES
>      imply TEST_DEVICES
>      imply VFIO_AMD_XGBE

What does this actually do ? Why should the choice of
accelerator affect what boards we pull in by default?
I can see why you'd want to disable boards that only
work with accelerators we don't enable, ie don't build
TCG-only boards unless CONFIG_TCG, but this is the other
way around...

thanks
-- PMM
Peter Maydell Sept. 29, 2020, 8:11 p.m. UTC | #4 
On Tue, 29 Sep 2020 at 21:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 16 Mar 2020 at 16:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > When building a KVM-only QEMU, the 'virt' machine is a good
> > default :)
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >  hw/arm/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index d0903d8544..8e801cd15f 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -1,5 +1,6 @@
> >  config ARM_VIRT
> >      bool
> > +    default y if KVM
> >      imply PCI_DEVICES
> >      imply TEST_DEVICES
> >      imply VFIO_AMD_XGBE
>
> What does this actually do ? Why should the choice of
> accelerator affect what boards we pull in by default?

Put another way, our current default is "build everything",
so "default y if ..." on a board is a no-op...

-- PMM
Philippe Mathieu-Daudé Sept. 29, 2020, 8:36 p.m. UTC | #5 
On 9/29/20 10:11 PM, Peter Maydell wrote:
> On Tue, 29 Sep 2020 at 21:06, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Mon, 16 Mar 2020 at 16:08, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> When building a KVM-only QEMU, the 'virt' machine is a good
>>> default :)
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>  hw/arm/Kconfig | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
>>> index d0903d8544..8e801cd15f 100644
>>> --- a/hw/arm/Kconfig
>>> +++ b/hw/arm/Kconfig
>>> @@ -1,5 +1,6 @@
>>>  config ARM_VIRT
>>>      bool
>>> +    default y if KVM
>>>      imply PCI_DEVICES
>>>      imply TEST_DEVICES
>>>      imply VFIO_AMD_XGBE
>>
>> What does this actually do ? Why should the choice of
>> accelerator affect what boards we pull in by default?
> 
> Put another way, our current default is "build everything",
> so "default y if ..." on a board is a no-op...

Yes, the problem if I don't restrict to KVM, when
using the Xen accelerator odd things occur
(using configure --enable-xen --disable-tcg --disable-kvm):

Compiling C object libqemu-i386-softmmu.fa.p/hw_cpu_a15mpcore.c.o
hw/cpu/a15mpcore.c:28:10: fatal error: kvm_arm.h: No such file or directory

See
https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Use_of_qemu-system-i386_on_ARM

We can't have the 'virt' machine automatically selected if
Xen is the only accelerator...

I'm looking for a simple way to avoid modifying the Xen code.

> 
> -- PMM
>
Paolo Bonzini Oct. 1, 2020, 7:38 a.m. UTC | #6 
On 29/09/20 22:36, Philippe Mathieu-Daudé wrote:
> Yes, the problem if I don't restrict to KVM, when
> using the Xen accelerator odd things occur
> (using configure --enable-xen --disable-tcg --disable-kvm):
> 
> Compiling C object libqemu-i386-softmmu.fa.p/hw_cpu_a15mpcore.c.o
> hw/cpu/a15mpcore.c:28:10: fatal error: kvm_arm.h: No such file or directory
> 
> See
> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Use_of_qemu-system-i386_on_ARM

I don't understand.  Is Xen adding CONFIG_ARM_VIRT=y to
default-configs/i386-softmmu.mak??

(By the way, there are duplicate Kconfig symbols between target/arm and
hw/cpu, they could/should be removed from target/arm).

Paolo
Philippe Mathieu-Daudé Oct. 1, 2020, 3:05 p.m. UTC | #7 
On 10/1/20 9:38 AM, Paolo Bonzini wrote:
> On 29/09/20 22:36, Philippe Mathieu-Daudé wrote:
>> Yes, the problem if I don't restrict to KVM, when
>> using the Xen accelerator odd things occur
>> (using configure --enable-xen --disable-tcg --disable-kvm):
>>
>> Compiling C object libqemu-i386-softmmu.fa.p/hw_cpu_a15mpcore.c.o
>> hw/cpu/a15mpcore.c:28:10: fatal error: kvm_arm.h: No such file or directory
>>
>> See
>> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Use_of_qemu-system-i386_on_ARM
> 
> I don't understand.  Is Xen adding CONFIG_ARM_VIRT=y to
> default-configs/i386-softmmu.mak??

No, this is when using:

 config ARM_VIRT
     bool
+    default y

I had the understanding devices in hw/$BASEARCH would be only
included for $ARCH, but I was wrong, any arch kconfig-include
the devices of the other archs.

I tried the following diff which doesn't build because various
devices in *non*-archdep folders use arch-specific Kconfig values:

-- >8 --
diff --git a/meson.build b/meson.build
index 9ab5d514d7..cfe19d0007 100644
--- a/meson.build
+++ b/meson.build
@@ -575,6 +575,7 @@ foreach target : target_dirs
     if fs.is_file(target_kconfig)
       minikconf_input += [target_kconfig]
     endif
+    minikconf_input += 'hw' / config_target['TARGET_BASE_ARCH'] / 'Kconfig'
     config_devices_mak = configure_file(
       input: minikconf_input,
       output: config_devices_mak,
diff --git a/hw/Kconfig b/hw/Kconfig
index 4de1797ffd..64c120175a 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -41,29 +41,29 @@ source vfio/Kconfig
 source watchdog/Kconfig

 # arch Kconfig
-source arm/Kconfig
-source alpha/Kconfig
-source avr/Kconfig
-source cris/Kconfig
-source hppa/Kconfig
-source i386/Kconfig
-source lm32/Kconfig
-source m68k/Kconfig
-source microblaze/Kconfig
-source mips/Kconfig
-source moxie/Kconfig
-source nios2/Kconfig
-source openrisc/Kconfig
-source ppc/Kconfig
-source riscv/Kconfig
-source rx/Kconfig
-source s390x/Kconfig
-source sh4/Kconfig
-source sparc/Kconfig
-source sparc64/Kconfig
-source tricore/Kconfig
-source unicore32/Kconfig
-source xtensa/Kconfig

 # Symbols used by multiple targets
 config TEST_DEVICES
---

> 
> (By the way, there are duplicate Kconfig symbols between target/arm and
> hw/cpu, they could/should be removed from target/arm).

I'd rather define Kconfig entry where the model is, so in this case
keep them defined in hw/cpu/Kconfig and remove dup entries from
hw/arm/Kconfig (if Peter is OK with that).

> 
> Paolo
>
Philippe Mathieu-Daudé Oct. 5, 2020, 9:22 a.m. UTC | #8 
On 10/1/20 5:05 PM, Philippe Mathieu-Daudé wrote:
> On 10/1/20 9:38 AM, Paolo Bonzini wrote:
>> On 29/09/20 22:36, Philippe Mathieu-Daudé wrote:
>>> Yes, the problem if I don't restrict to KVM, when
>>> using the Xen accelerator odd things occur
>>> (using configure --enable-xen --disable-tcg --disable-kvm):
>>>
>>> Compiling C object libqemu-i386-softmmu.fa.p/hw_cpu_a15mpcore.c.o
>>> hw/cpu/a15mpcore.c:28:10: fatal error: kvm_arm.h: No such file or directory
>>>
>>> See
>>> https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Use_of_qemu-system-i386_on_ARM
>>
>> I don't understand.  Is Xen adding CONFIG_ARM_VIRT=y to
>> default-configs/i386-softmmu.mak??
> 
> No, this is when using:
> 
>  config ARM_VIRT
>      bool
> +    default y
> 
> I had the understanding devices in hw/$BASEARCH would be only
> included for $ARCH, but I was wrong, any arch kconfig-include
> the devices of the other archs.
> 
> I tried the following diff which doesn't build because various
> devices in *non*-archdep folders use arch-specific Kconfig values:
> 
> -- >8 --
> diff --git a/meson.build b/meson.build
> index 9ab5d514d7..cfe19d0007 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -575,6 +575,7 @@ foreach target : target_dirs
>      if fs.is_file(target_kconfig)
>        minikconf_input += [target_kconfig]
>      endif
> +    minikconf_input += 'hw' / config_target['TARGET_BASE_ARCH'] / 'Kconfig'
>      config_devices_mak = configure_file(
>        input: minikconf_input,
>        output: config_devices_mak,
> diff --git a/hw/Kconfig b/hw/Kconfig
> index 4de1797ffd..64c120175a 100644
> --- a/hw/Kconfig
> +++ b/hw/Kconfig
> @@ -41,29 +41,29 @@ source vfio/Kconfig
>  source watchdog/Kconfig
> 
>  # arch Kconfig
> -source arm/Kconfig
> -source alpha/Kconfig
> -source avr/Kconfig
> -source cris/Kconfig
> -source hppa/Kconfig
> -source i386/Kconfig
> -source lm32/Kconfig
> -source m68k/Kconfig
> -source microblaze/Kconfig
> -source mips/Kconfig
> -source moxie/Kconfig
> -source nios2/Kconfig
> -source openrisc/Kconfig
> -source ppc/Kconfig
> -source riscv/Kconfig
> -source rx/Kconfig
> -source s390x/Kconfig
> -source sh4/Kconfig
> -source sparc/Kconfig
> -source sparc64/Kconfig
> -source tricore/Kconfig
> -source unicore32/Kconfig
> -source xtensa/Kconfig
> 
>  # Symbols used by multiple targets
>  config TEST_DEVICES
> ---

List of arch-indep Kconfig using arch-defined selectors:

hw/acpi/Kconfig:42:    depends on PC
hw/intc/Kconfig:31:    depends on ARM_GIC && KVM
hw/intc/Kconfig:36:    depends on OPENPIC && KVM
hw/intc/Kconfig:40:    depends on POWERNV || PSERIES
hw/intc/Kconfig:49:    depends on XICS && KVM
hw/intc/Kconfig:60:    depends on S390_FLIC && KVM
hw/mem/Kconfig:11:    depends on (PC || PSERIES || ARM_VIRT)
hw/pci-bridge/Kconfig:8:    default y if Q35
hw/timer/Kconfig:14:    default y if PC
hw/tpm/Kconfig:18:    depends on TPM && PC
hw/tpm/Kconfig:24:    depends on TPM && PSERIES
hw/vfio/Kconfig:16:    depends on LINUX && S390_CCW_VIRTIO
hw/vfio/Kconfig:38:    depends on LINUX && S390_CCW_VIRTIO
Paolo Bonzini Oct. 5, 2020, 10:53 a.m. UTC | #9 
On 05/10/20 11:22, Philippe Mathieu-Daudé wrote:
> List of arch-indep Kconfig using arch-defined selectors:
> 
> hw/acpi/Kconfig:42:    depends on PC
> hw/intc/Kconfig:31:    depends on ARM_GIC && KVM
> hw/intc/Kconfig:36:    depends on OPENPIC && KVM
> hw/intc/Kconfig:40:    depends on POWERNV || PSERIES
> hw/intc/Kconfig:49:    depends on XICS && KVM
> hw/intc/Kconfig:60:    depends on S390_FLIC && KVM
> hw/mem/Kconfig:11:    depends on (PC || PSERIES || ARM_VIRT)
> hw/pci-bridge/Kconfig:8:    default y if Q35
> hw/timer/Kconfig:14:    default y if PC
> hw/tpm/Kconfig:18:    depends on TPM && PC
> hw/tpm/Kconfig:24:    depends on TPM && PSERIES
> hw/vfio/Kconfig:16:    depends on LINUX && S390_CCW_VIRTIO
> hw/vfio/Kconfig:38:    depends on LINUX && S390_CCW_VIRTIO
> 

I don't think that's a problem, and also I'm not sure this patch is a
good idea.

See docs/devel/kconfig.rst: "Boards default to false; they are enabled
by the ``default-configs/*.mak`` for the target they apply to".

Paolo
diff mbox series

Patch

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index d0903d8544..8e801cd15f 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -1,5 +1,6 @@ 
 config ARM_VIRT
     bool
+    default y if KVM
     imply PCI_DEVICES
     imply TEST_DEVICES
     imply VFIO_AMD_XGBE