[PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Julia Suvorova]

In the ACPI specification [1], the 'unarmed' bit is set when a device
cannot accept a persistent write. This means that when a memdev is
read-only, the 'unarmed' flag must be turned on. The logic is correct,
just changing the error message.

[1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3

Signed-off-by: Julia Suvorova <jusual@redhat.com>
---
 hw/mem/nvdimm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 7c7d777781..bfb76818c1 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -149,7 +149,7 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
     if (!nvdimm->unarmed && memory_region_is_rom(mr)) {
         HostMemoryBackend *hostmem = dimm->hostmem;
 
-        error_setg(errp, "'unarmed' property must be off since memdev %s "
+        error_setg(errp, "'unarmed' property must be on since memdev %s "
                    "is read-only",
                    object_get_canonical_path_component(OBJECT(hostmem)));
         return;
-- 
2.35.1

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Stefan Hajnoczi]

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

On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:
> In the ACPI specification [1], the 'unarmed' bit is set when a device
> cannot accept a persistent write. This means that when a memdev is
> read-only, the 'unarmed' flag must be turned on. The logic is correct,
> just changing the error message.
> 
> [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> 
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> ---
>  hw/mem/nvdimm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Julia Suvorova]

On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:
> > In the ACPI specification [1], the 'unarmed' bit is set when a device
> > cannot accept a persistent write. This means that when a memdev is
> > read-only, the 'unarmed' flag must be turned on. The logic is correct,
> > just changing the error message.
> >
> > [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > ---
> >  hw/mem/nvdimm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

It seems like Xiao is not active, whose tree should this patch go to?

Best regards, Julia Suvorova.

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Stefan Hajnoczi]

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

On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:
> On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:
> > > In the ACPI specification [1], the 'unarmed' bit is set when a device
> > > cannot accept a persistent write. This means that when a memdev is
> > > read-only, the 'unarmed' flag must be turned on. The logic is correct,
> > > just changing the error message.
> > >
> > > [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> > >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > ---
> > >  hw/mem/nvdimm.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> It seems like Xiao is not active, whose tree should this patch go to?

Michael or Igor can merge it:

  $ scripts/get_maintainer.pl -f hw/mem/nvdimm.c
  Xiao Guangrong <xiaoguangrong.eric@gmail.com> (maintainer:NVDIMM)
  "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
  Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
  Ani Sinha <ani@anisinha.ca> (reviewer:ACPI/SMBIOS)
  qemu-devel@nongnu.org (open list:All patches CC here)

Stefan

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

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Igor Mammedov]

On Mon, 13 Jun 2022 16:09:53 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:
> > On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:  
> > >
> > > On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:  
> > > > In the ACPI specification [1], the 'unarmed' bit is set when a device
> > > > cannot accept a persistent write. This means that when a memdev is
> > > > read-only, the 'unarmed' flag must be turned on. The logic is correct,
> > > > just changing the error message.
> > > >
> > > > [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> > > >
> > > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > > ---
> > > >  hw/mem/nvdimm.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)  
> > >
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>  
> > 
> > It seems like Xiao is not active, whose tree should this patch go to?  

Perhaps David can add himself as maintainer (i.e. put it
under memory mantanership umbrella) and merge it 

> 
> Michael or Igor can merge it:
> 
>   $ scripts/get_maintainer.pl -f hw/mem/nvdimm.c
>   Xiao Guangrong <xiaoguangrong.eric@gmail.com> (maintainer:NVDIMM)
>   "Michael S. Tsirkin" <mst@redhat.com> (supporter:ACPI/SMBIOS)
>   Igor Mammedov <imammedo@redhat.com> (supporter:ACPI/SMBIOS)
>   Ani Sinha <ani@anisinha.ca> (reviewer:ACPI/SMBIOS)
>   qemu-devel@nongnu.org (open list:All patches CC here)
> 
> Stefan

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[David Hildenbrand]

On 14.06.22 10:54, Igor Mammedov wrote:
> On Mon, 13 Jun 2022 16:09:53 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
>> On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:
>>> On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:  
>>>>
>>>> On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:  
>>>>> In the ACPI specification [1], the 'unarmed' bit is set when a device
>>>>> cannot accept a persistent write. This means that when a memdev is
>>>>> read-only, the 'unarmed' flag must be turned on. The logic is correct,
>>>>> just changing the error message.
>>>>>
>>>>> [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
>>>>>
>>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
>>>>> ---
>>>>>  hw/mem/nvdimm.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)  
>>>>
>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>  
>>>
>>> It seems like Xiao is not active, whose tree should this patch go to?  

Is that a temporary or a permanent thing? Do we know?

> 
> Perhaps David can add himself as maintainer (i.e. put it
> under memory mantanership umbrella) and merge it 

Maybe it makes sense to combine NVDIMM with pc-dimm.c and
memory-device.c into a "MEMORY DEVICE" section. Then, remove "hw/mem/*"
from "ACPI/SMBIOS".

cxl_type3.c, npcm7xx_mc.c and sparse-mem.c in /hw/mem/ are a bit
different. We could add cxl_type3.c to "Compute Express Link".
npcm7xx_mc.c and sparse-mem.c should be already covered.

-- 
Thanks,

David / dhildenb

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Julia Suvorova]

On Tue, Jun 14, 2022 at 11:50 AM David Hildenbrand <david@redhat.com> wrote:
>
> On 14.06.22 10:54, Igor Mammedov wrote:
> > On Mon, 13 Jun 2022 16:09:53 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> >> On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:
> >>> On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >>>>
> >>>> On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:
> >>>>> In the ACPI specification [1], the 'unarmed' bit is set when a device
> >>>>> cannot accept a persistent write. This means that when a memdev is
> >>>>> read-only, the 'unarmed' flag must be turned on. The logic is correct,
> >>>>> just changing the error message.
> >>>>>
> >>>>> [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> >>>>>
> >>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> >>>>> ---
> >>>>>  hw/mem/nvdimm.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>>
> >>> It seems like Xiao is not active, whose tree should this patch go to?
>
> Is that a temporary or a permanent thing? Do we know?

No idea. But his last signed-off was three years ago.

> >
> > Perhaps David can add himself as maintainer (i.e. put it
> > under memory mantanership umbrella) and merge it
>
> Maybe it makes sense to combine NVDIMM with pc-dimm.c and
> memory-device.c into a "MEMORY DEVICE" section. Then, remove "hw/mem/*"
> from "ACPI/SMBIOS".
>
> cxl_type3.c, npcm7xx_mc.c and sparse-mem.c in /hw/mem/ are a bit
> different. We could add cxl_type3.c to "Compute Express Link".
> npcm7xx_mc.c and sparse-mem.c should be already covered.
>
> --
> Thanks,
>
> David / dhildenb
>

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[David Hildenbrand]

On 14.06.22 14:13, Julia Suvorova wrote:
> On Tue, Jun 14, 2022 at 11:50 AM David Hildenbrand <david@redhat.com> wrote:
>>
>> On 14.06.22 10:54, Igor Mammedov wrote:
>>> On Mon, 13 Jun 2022 16:09:53 +0100
>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>
>>>> On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:
>>>>> On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>>>>
>>>>>> On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:
>>>>>>> In the ACPI specification [1], the 'unarmed' bit is set when a device
>>>>>>> cannot accept a persistent write. This means that when a memdev is
>>>>>>> read-only, the 'unarmed' flag must be turned on. The logic is correct,
>>>>>>> just changing the error message.
>>>>>>>
>>>>>>> [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
>>>>>>>
>>>>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
>>>>>>> ---
>>>>>>>  hw/mem/nvdimm.c | 2 +-
>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>>
>>>>> It seems like Xiao is not active, whose tree should this patch go to?
>>
>> Is that a temporary or a permanent thing? Do we know?
> 
> No idea. But his last signed-off was three years ago.

I sent a patch to Xiao, asking if he's still active in QEMU. If I don't
get a reply this week, I'll move forward with proposing an update to
MAINTAINERS as described.

-- 
Thanks,

David / dhildenb

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Xiao Guangrong]

On Wed, Jun 15, 2022 at 4:24 PM David Hildenbrand <david@redhat.com> wrote:

> >> Is that a temporary or a permanent thing? Do we know?
> >
> > No idea. But his last signed-off was three years ago.
>
> I sent a patch to Xiao, asking if he's still active in QEMU. If I don't
> get a reply this week, I'll move forward with proposing an update to
> MAINTAINERS as described.
>

Okay, please do it.

Sorry, I am just roughly reading the mailing list of qemu & kvm usually,
and do not get enough time to actively review or contribute on these
fields. :-(

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[David Hildenbrand]

On 15.06.22 13:17, Xiao Guangrong wrote:
> On Wed, Jun 15, 2022 at 4:24 PM David Hildenbrand <david@redhat.com> wrote:
> 
>>>> Is that a temporary or a permanent thing? Do we know?
>>>
>>> No idea. But his last signed-off was three years ago.
>>
>> I sent a patch to Xiao, asking if he's still active in QEMU. If I don't

s/patch/mail/ :)

>> get a reply this week, I'll move forward with proposing an update to
>> MAINTAINERS as described.
>>
> 
> Okay, please do it.
> 
> Sorry, I am just roughly reading the mailing list of qemu & kvm usually,
> and do not get enough time to actively review or contribute on these
> fields. :-(

Not an issue, thanks for that information and thanks for your work in
the past on that!

Should I keep you entered as a reviewer for the new section?

-- 
Thanks,

David / dhildenb

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Xiao Guangrong]

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

On Wed, Jun 15, 2022 at 7:49 PM David Hildenbrand <david@redhat.com> wrote:
>
> On 15.06.22 13:17, Xiao Guangrong wrote:
> > On Wed, Jun 15, 2022 at 4:24 PM David Hildenbrand <david@redhat.com>
wrote:
> >
> >>>> Is that a temporary or a permanent thing? Do we know?
> >>>
> >>> No idea. But his last signed-off was three years ago.
> >>
> >> I sent a patch to Xiao, asking if he's still active in QEMU. If I don't
>
> s/patch/mail/ :)
>
> >> get a reply this week, I'll move forward with proposing an update to
> >> MAINTAINERS as described.
> >>
> >
> > Okay, please do it.
> >
> > Sorry, I am just roughly reading the mailing list of qemu & kvm usually,
> > and do not get enough time to actively review or contribute on these
> > fields. :-(
>
> Not an issue, thanks for that information and thanks for your work in
> the past on that!
>
> Should I keep you entered as a reviewer for the new section?

Okay, that is good for me! :)

[-- Attachment #2: Type: text/html, Size: 1326 bytes --]

Re: [PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag

[Igor Mammedov]

On Tue, 14 Jun 2022 11:50:43 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 14.06.22 10:54, Igor Mammedov wrote:
> > On Mon, 13 Jun 2022 16:09:53 +0100
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >   
> >> On Mon, Jun 13, 2022 at 05:01:10PM +0200, Julia Suvorova wrote:  
> >>> On Tue, May 31, 2022 at 5:32 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:    
> >>>>
> >>>> On Tue, May 31, 2022 at 04:51:47PM +0200, Julia Suvorova wrote:    
> >>>>> In the ACPI specification [1], the 'unarmed' bit is set when a device
> >>>>> cannot accept a persistent write. This means that when a memdev is
> >>>>> read-only, the 'unarmed' flag must be turned on. The logic is correct,
> >>>>> just changing the error message.
> >>>>>
> >>>>> [1] ACPI NFIT NVDIMM Region Mapping Structure "NVDIMM State Flags" Bit 3
> >>>>>
> >>>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> >>>>> ---
> >>>>>  hw/mem/nvdimm.c | 2 +-
> >>>>>  1 file changed, 1 insertion(+), 1 deletion(-)    
> >>>>
> >>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>    
> >>>
> >>> It seems like Xiao is not active, whose tree should this patch go to?    
> 
> Is that a temporary or a permanent thing? Do we know?
> 
> > 
> > Perhaps David can add himself as maintainer (i.e. put it
> > under memory mantanership umbrella) and merge it   
> 
> Maybe it makes sense to combine NVDIMM with pc-dimm.c and
> memory-device.c into a "MEMORY DEVICE" section. Then, remove "hw/mem/*"
> from "ACPI/SMBIOS".
just keep me on supporter list for them so I won't miss
patches that needs reviewing.

> cxl_type3.c, npcm7xx_mc.c and sparse-mem.c in /hw/mem/ are a bit
> different. We could add cxl_type3.c to "Compute Express Link".
> npcm7xx_mc.c and sparse-mem.c should be already covered. 
for cxl I'd add Michael as it's mostly all PCI stuff