[PATCH] hw/ppc/Kconfig: NVDIMM is a hard requirement for the pseries machine

Thomas Huth posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230504180521.220404-1-thuth@redhat.com
hw/ppc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hw/ppc/Kconfig: NVDIMM is a hard requirement for the pseries machine
Posted by Thomas Huth 1 year ago
When building QEMU with "--without-default-devices", the pseries
machine fails to start even when running with the --nodefaults option:

 $ ./qemu-system-ppc64 --nodefaults -M pseries
 Type 'spapr-nvdimm' is missing its parent 'nvdimm'
 Aborted (core dumped)

Looks like NVDIMM is a hard requirement for this machine nowadays.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index c898021b5f..a689d9b219 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -3,7 +3,7 @@ config PSERIES
     imply PCI_DEVICES
     imply TEST_DEVICES
     imply VIRTIO_VGA
-    imply NVDIMM
+    select NVDIMM
     select DIMM
     select PCI
     select SPAPR_VSCSI
-- 
2.31.1
Re: [PATCH] hw/ppc/Kconfig: NVDIMM is a hard requirement for the pseries machine
Posted by Daniel Henrique Barboza 1 year ago

On 5/4/23 15:05, Thomas Huth wrote:
> When building QEMU with "--without-default-devices", the pseries
> machine fails to start even when running with the --nodefaults option:
> 
>   $ ./qemu-system-ppc64 --nodefaults -M pseries
>   Type 'spapr-nvdimm' is missing its parent 'nvdimm'
>   Aborted (core dumped)
> 
> Looks like NVDIMM is a hard requirement for this machine nowadays.

Ouch.

I believe this has to do with this comment in hw/ppc/spapr.c, in
spapr_instance_init():

     /*
      * NVDIMM support went live in 5.1 without considering that, in
      * other archs, the user needs to enable NVDIMM support with the
      * 'nvdimm' machine option and the default behavior is NVDIMM
      * support disabled. It is too late to roll back to the standard
      * behavior without breaking 5.1 guests.
      */
     if (mc->nvdimm_supported) {
         ms->nvdimms_state->is_enabled = true;
     }

It seems like you found out another side effect of this nvdimm situation that Igor
documented 2 years ago in 55810e90 ("ppc/spapr: cleanup -machine pseries,nvdimm=X
handling").


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


I'll send a PPC PR in the next few days. Let me know if you want me to queue it.


Thanks,


Daniel

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/ppc/Kconfig | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index c898021b5f..a689d9b219 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -3,7 +3,7 @@ config PSERIES
>       imply PCI_DEVICES
>       imply TEST_DEVICES
>       imply VIRTIO_VGA
> -    imply NVDIMM
> +    select NVDIMM
>       select DIMM
>       select PCI
>       select SPAPR_VSCSI
Re: [PATCH] hw/ppc/Kconfig: NVDIMM is a hard requirement for the pseries machine
Posted by Thomas Huth 1 year ago
On 04/05/2023 23.19, Daniel Henrique Barboza wrote:
> 
> 
> On 5/4/23 15:05, Thomas Huth wrote:
>> When building QEMU with "--without-default-devices", the pseries
>> machine fails to start even when running with the --nodefaults option:
>>
>>   $ ./qemu-system-ppc64 --nodefaults -M pseries
>>   Type 'spapr-nvdimm' is missing its parent 'nvdimm'
>>   Aborted (core dumped)
>>
>> Looks like NVDIMM is a hard requirement for this machine nowadays.
> 
> Ouch.
> 
> I believe this has to do with this comment in hw/ppc/spapr.c, in
> spapr_instance_init():
> 
>      /*
>       * NVDIMM support went live in 5.1 without considering that, in
>       * other archs, the user needs to enable NVDIMM support with the
>       * 'nvdimm' machine option and the default behavior is NVDIMM
>       * support disabled. It is too late to roll back to the standard
>       * behavior without breaking 5.1 guests.
>       */
>      if (mc->nvdimm_supported) {
>          ms->nvdimms_state->is_enabled = true;
>      }
> 
> It seems like you found out another side effect of this nvdimm situation 
> that Igor
> documented 2 years ago in 55810e90 ("ppc/spapr: cleanup -machine 
> pseries,nvdimm=X
> handling").
> 
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> 
> 
> I'll send a PPC PR in the next few days. Let me know if you want me to queue 
> it.

Yes, please add it to your queue!

  Thanks,
   Thomas


Re: [PATCH] hw/ppc/Kconfig: NVDIMM is a hard requirement for the pseries machine
Posted by Daniel Henrique Barboza 1 year ago

On 5/5/23 05:03, Thomas Huth wrote:
> On 04/05/2023 23.19, Daniel Henrique Barboza wrote:
>>
>>
>> On 5/4/23 15:05, Thomas Huth wrote:
>>> When building QEMU with "--without-default-devices", the pseries
>>> machine fails to start even when running with the --nodefaults option:
>>>
>>>   $ ./qemu-system-ppc64 --nodefaults -M pseries
>>>   Type 'spapr-nvdimm' is missing its parent 'nvdimm'
>>>   Aborted (core dumped)
>>>
>>> Looks like NVDIMM is a hard requirement for this machine nowadays.
>>
>> Ouch.
>>
>> I believe this has to do with this comment in hw/ppc/spapr.c, in
>> spapr_instance_init():
>>
>>      /*
>>       * NVDIMM support went live in 5.1 without considering that, in
>>       * other archs, the user needs to enable NVDIMM support with the
>>       * 'nvdimm' machine option and the default behavior is NVDIMM
>>       * support disabled. It is too late to roll back to the standard
>>       * behavior without breaking 5.1 guests.
>>       */
>>      if (mc->nvdimm_supported) {
>>          ms->nvdimms_state->is_enabled = true;
>>      }
>>
>> It seems like you found out another side effect of this nvdimm situation that Igor
>> documented 2 years ago in 55810e90 ("ppc/spapr: cleanup -machine pseries,nvdimm=X
>> handling").
>>
>>
>> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>
>>
>> I'll send a PPC PR in the next few days. Let me know if you want me to queue it.
> 
> Yes, please add it to your queue!

Queued. Thanks,


Daniel

> 
>   Thanks,
>    Thomas
>