* [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
@ 2022-10-18 15:25 Julia Suvorova
2022-10-18 16:17 ` Philippe Mathieu-Daudé
2022-10-20 5:17 ` Pankaj Gupta
0 siblings, 2 replies; 6+ messages in thread
From: Julia Suvorova @ 2022-10-18 15:25 UTC (permalink / raw)
To: qemu-devel
Cc: Michael S. Tsirkin, Igor Mammedov, David Hildenbrand,
Julia Suvorova, Stefan Hajnoczi
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>
Reviewed-by: Stefan Hajnoczi <stefanha@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.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
2022-10-18 15:25 [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag Julia Suvorova
@ 2022-10-18 16:17 ` Philippe Mathieu-Daudé
2022-10-18 16:49 ` Michael S. Tsirkin
2022-10-20 5:17 ` Pankaj Gupta
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-18 16:17 UTC (permalink / raw)
To: Julia Suvorova, qemu-devel
Cc: Michael S. Tsirkin, Igor Mammedov, David Hildenbrand,
Stefan Hajnoczi
On 18/10/22 17:25, 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
>
Fixes: dbd730e859 ("nvdimm: check -object memory-backend-file,
readonly=on option")
The documentation in 'docs/nvdimm.txt' is correct :)
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@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 "
If you ever respin please quote 'on' for readability.
> "is read-only",
> object_get_canonical_path_component(OBJECT(hostmem)));
> return;
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
2022-10-18 16:17 ` Philippe Mathieu-Daudé
@ 2022-10-18 16:49 ` Michael S. Tsirkin
2022-10-19 12:40 ` David Hildenbrand
2022-10-19 19:31 ` Julia Suvorova
0 siblings, 2 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2022-10-18 16:49 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Julia Suvorova, qemu-devel, Igor Mammedov, David Hildenbrand,
Stefan Hajnoczi
On Tue, Oct 18, 2022 at 06:17:55PM +0200, Philippe Mathieu-Daudé wrote:
> On 18/10/22 17:25, 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
> >
>
> Fixes: dbd730e859 ("nvdimm: check -object memory-backend-file, readonly=on
> option")
>
> The documentation in 'docs/nvdimm.txt' is correct :)
>
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@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 "
>
> If you ever respin please quote 'on' for readability.
Yes make sense. Julia could you change this pls?
> > "is read-only",
> > object_get_canonical_path_component(OBJECT(hostmem)));
> > return;
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
2022-10-18 16:49 ` Michael S. Tsirkin
@ 2022-10-19 12:40 ` David Hildenbrand
2022-10-19 19:31 ` Julia Suvorova
1 sibling, 0 replies; 6+ messages in thread
From: David Hildenbrand @ 2022-10-19 12:40 UTC (permalink / raw)
To: Michael S. Tsirkin, Philippe Mathieu-Daudé
Cc: Julia Suvorova, qemu-devel, Igor Mammedov, Stefan Hajnoczi
On 18.10.22 18:49, Michael S. Tsirkin wrote:
> On Tue, Oct 18, 2022 at 06:17:55PM +0200, Philippe Mathieu-Daudé wrote:
>> On 18/10/22 17:25, 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
>>>
>>
>> Fixes: dbd730e859 ("nvdimm: check -object memory-backend-file, readonly=on
>> option")
>>
>> The documentation in 'docs/nvdimm.txt' is correct :)
>>
>>> Signed-off-by: Julia Suvorova <jusual@redhat.com>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@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 "
>>
>> If you ever respin please quote 'on' for readability.
>
>
> Yes make sense. Julia could you change this pls?
Acked-by: David Hildenbrand <david@redhat.com>
I can pick this up once resent. I'll most probably send a merge request
by the end of next week.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
2022-10-18 16:49 ` Michael S. Tsirkin
2022-10-19 12:40 ` David Hildenbrand
@ 2022-10-19 19:31 ` Julia Suvorova
1 sibling, 0 replies; 6+ messages in thread
From: Julia Suvorova @ 2022-10-19 19:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Philippe Mathieu-Daudé, qemu-devel, Igor Mammedov,
David Hildenbrand, Stefan Hajnoczi
On Tue, Oct 18, 2022 at 6:49 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Oct 18, 2022 at 06:17:55PM +0200, Philippe Mathieu-Daudé wrote:
> > On 18/10/22 17:25, 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
> > >
> >
> > Fixes: dbd730e859 ("nvdimm: check -object memory-backend-file, readonly=on
> > option")
> >
> > The documentation in 'docs/nvdimm.txt' is correct :)
> >
> > > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > > Reviewed-by: Stefan Hajnoczi <stefanha@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 "
> >
> > If you ever respin please quote 'on' for readability.
>
>
> Yes make sense. Julia could you change this pls?
Sure, will do.
> > > "is read-only",
> > > object_get_canonical_path_component(OBJECT(hostmem)));
> > > return;
> >
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag
2022-10-18 15:25 [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag Julia Suvorova
2022-10-18 16:17 ` Philippe Mathieu-Daudé
@ 2022-10-20 5:17 ` Pankaj Gupta
1 sibling, 0 replies; 6+ messages in thread
From: Pankaj Gupta @ 2022-10-20 5:17 UTC (permalink / raw)
To: Julia Suvorova
Cc: qemu-devel, Michael S. Tsirkin, Igor Mammedov, David Hildenbrand,
Stefan Hajnoczi
> 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>
> Reviewed-by: Stefan Hajnoczi <stefanha@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;
With the suggested minor change.
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-20 5:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-18 15:25 [RESEND PATCH] hw/mem/nvdimm: fix error message for 'unarmed' flag Julia Suvorova
2022-10-18 16:17 ` Philippe Mathieu-Daudé
2022-10-18 16:49 ` Michael S. Tsirkin
2022-10-19 12:40 ` David Hildenbrand
2022-10-19 19:31 ` Julia Suvorova
2022-10-20 5:17 ` Pankaj Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).