| Message ID | 20220531145147.61112-1-jusual@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/mem/nvdimm: fix error message for 'unarmed' flag | expand |
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>
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.
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
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
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.
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 >
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
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.
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. :-(
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?
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! :)
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;
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(-)