* [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree
@ 2012-02-03 12:02 Jan Kiszka
2012-02-04 12:12 ` Blue Swirl
2012-02-04 14:57 ` [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only " Jan Kiszka
0 siblings, 2 replies; 12+ messages in thread
From: Jan Kiszka @ 2012-02-03 12:02 UTC (permalink / raw)
To: qemu-trivial; +Cc: Blue Swirl, qemu-devel, Avi Kivity
Helpful to understand guest configurations of things like the i440FX's
PAM.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
memory.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/memory.c b/memory.c
index ee4c98a..ea4adda 100644
--- a/memory.c
+++ b/memory.c
@@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
ml->printed = false;
QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
}
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
+ mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readonly ? "RO" : "RW",
mr->name,
mr->alias->name,
mr->alias_offset,
mr->alias_offset
+ (target_phys_addr_t)int128_get64(mr->size) - 1);
} else {
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
+ mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readonly ? "RO" : "RW",
mr->name);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-03 12:02 [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree Jan Kiszka
@ 2012-02-04 12:12 ` Blue Swirl
2012-02-04 12:23 ` Jan Kiszka
2012-02-04 14:57 ` [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only " Jan Kiszka
1 sibling, 1 reply; 12+ messages in thread
From: Blue Swirl @ 2012-02-04 12:12 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-trivial, qemu-devel, Avi Kivity
On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Helpful to understand guest configurations of things like the i440FX's
> PAM.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> memory.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index ee4c98a..ea4adda 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
> ml->printed = false;
> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
> }
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readonly ? "RO" : "RW",
I think the reserved regions which are unreadable and unwritable
should be shown as well. Then the output should be a combination of
'R', 'W' or neither ('-').
> mr->name,
> mr->alias->name,
> mr->alias_offset,
> mr->alias_offset
> + (target_phys_addr_t)int128_get64(mr->size) - 1);
> } else {
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readonly ? "RO" : "RW",
> mr->name);
> }
>
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-04 12:12 ` Blue Swirl
@ 2012-02-04 12:23 ` Jan Kiszka
2012-02-04 12:32 ` Blue Swirl
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2012-02-04 12:23 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-trivial, qemu-devel, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]
On 2012-02-04 13:12, Blue Swirl wrote:
> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Helpful to understand guest configurations of things like the i440FX's
>> PAM.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> memory.c | 6 ++++--
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/memory.c b/memory.c
>> index ee4c98a..ea4adda 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
>> ml->printed = false;
>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>> }
>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
>> base + mr->addr,
>> base + mr->addr
>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
>> mr->priority,
>> + mr->readonly ? "RO" : "RW",
>
> I think the reserved regions which are unreadable and unwritable
> should be shown as well. Then the output should be a combination of
> 'R', 'W' or neither ('-').
Reserved regions are in the hand of some other device model (so far only
the KVM kernel). That says nothing about their R/W property. If we ever
have a reserved region that is not writable, the owner could still set
the corresponding flag for documentation purposes.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-04 12:23 ` Jan Kiszka
@ 2012-02-04 12:32 ` Blue Swirl
2012-02-04 14:51 ` Jan Kiszka
0 siblings, 1 reply; 12+ messages in thread
From: Blue Swirl @ 2012-02-04 12:32 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-trivial, qemu-devel, Avi Kivity
On Sat, Feb 4, 2012 at 12:23, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2012-02-04 13:12, Blue Swirl wrote:
>> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Helpful to understand guest configurations of things like the i440FX's
>>> PAM.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> memory.c | 6 ++++--
>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/memory.c b/memory.c
>>> index ee4c98a..ea4adda 100644
>>> --- a/memory.c
>>> +++ b/memory.c
>>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
>>> ml->printed = false;
>>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>>> }
>>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
>>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
>>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
>>> base + mr->addr,
>>> base + mr->addr
>>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
>>> mr->priority,
>>> + mr->readonly ? "RO" : "RW",
>>
>> I think the reserved regions which are unreadable and unwritable
>> should be shown as well. Then the output should be a combination of
>> 'R', 'W' or neither ('-').
>
> Reserved regions are in the hand of some other device model (so far only
> the KVM kernel). That says nothing about their R/W property. If we ever
> have a reserved region that is not writable, the owner could still set
> the corresponding flag for documentation purposes.
OK. But it's also possible for a region to have readable == false
while readonly == false, which would imply 'WO' or '-W'. That also
supports separate 'R', 'W' and '-' flags.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-04 12:32 ` Blue Swirl
@ 2012-02-04 14:51 ` Jan Kiszka
2012-02-09 8:35 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2012-02-04 14:51 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-trivial, qemu-devel, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]
On 2012-02-04 13:32, Blue Swirl wrote:
> On Sat, Feb 4, 2012 at 12:23, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2012-02-04 13:12, Blue Swirl wrote:
>>> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> Helpful to understand guest configurations of things like the i440FX's
>>>> PAM.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>> memory.c | 6 ++++--
>>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/memory.c b/memory.c
>>>> index ee4c98a..ea4adda 100644
>>>> --- a/memory.c
>>>> +++ b/memory.c
>>>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
>>>> ml->printed = false;
>>>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>>>> }
>>>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
>>>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
>>>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
>>>> base + mr->addr,
>>>> base + mr->addr
>>>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
>>>> mr->priority,
>>>> + mr->readonly ? "RO" : "RW",
>>>
>>> I think the reserved regions which are unreadable and unwritable
>>> should be shown as well. Then the output should be a combination of
>>> 'R', 'W' or neither ('-').
>>
>> Reserved regions are in the hand of some other device model (so far only
>> the KVM kernel). That says nothing about their R/W property. If we ever
>> have a reserved region that is not writable, the owner could still set
>> the corresponding flag for documentation purposes.
>
> OK. But it's also possible for a region to have readable == false
> while readonly == false, which would imply 'WO' or '-W'. That also
> supports separate 'R', 'W' and '-' flags.
Yep, I encoded the ROM device state as well. And this revealed a
regression of the memory region conversion of the cfi02. Gave up
counting how often I fixed this type of bug in the flash code.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only on info mtree
2012-02-03 12:02 [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree Jan Kiszka
2012-02-04 12:12 ` Blue Swirl
@ 2012-02-04 14:57 ` Jan Kiszka
2012-02-04 15:04 ` Blue Swirl
1 sibling, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2012-02-04 14:57 UTC (permalink / raw)
To: Blue Swirl, qemu-devel; +Cc: Avi Kivity
From: Jan Kiszka <jan.kiszka@siemens.com>
Helpful to understand guest configurations of things like the i440FX's
PAM or the state of ROM devices.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes in v2:
- encode R and W separately
- print ROMD memory region state as well
memory.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index 5e77d8a..3095391 100644
--- a/memory.c
+++ b/memory.c
@@ -1609,23 +1609,31 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
ml->printed = false;
QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
}
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
- TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
+ mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
+ " (prio %d, %c%c): alias %s @%s " TARGET_FMT_plx
+ "-" TARGET_FMT_plx "\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readable ? 'R' : '-',
+ !mr->readonly && (!mr->rom_device || !mr->readable) ? 'W'
+ : '-',
mr->name,
mr->alias->name,
mr->alias_offset,
mr->alias_offset
+ (target_phys_addr_t)int128_get64(mr->size) - 1);
} else {
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
+ mon_printf(f,
+ TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readable ? 'R' : '-',
+ !mr->readonly && (!mr->rom_device || !mr->readable) ? 'W'
+ : '-',
mr->name);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only on info mtree
2012-02-04 14:57 ` [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only " Jan Kiszka
@ 2012-02-04 15:04 ` Blue Swirl
2012-02-04 15:25 ` [Qemu-devel] [PATCH v3] " Jan Kiszka
0 siblings, 1 reply; 12+ messages in thread
From: Blue Swirl @ 2012-02-04 15:04 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Avi Kivity
On Sat, Feb 4, 2012 at 14:57, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Helpful to understand guest configurations of things like the i440FX's
> PAM or the state of ROM devices.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v2:
> - encode R and W separately
> - print ROMD memory region state as well
>
> memory.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index 5e77d8a..3095391 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1609,23 +1609,31 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
> ml->printed = false;
> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
> }
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
> - TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
> + " (prio %d, %c%c): alias %s @%s " TARGET_FMT_plx
> + "-" TARGET_FMT_plx "\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readable ? 'R' : '-',
> + !mr->readonly && (!mr->rom_device || !mr->readable) ? 'W'
> + : '-',
Why || !mr->readable? Applying de Morgan, this would equal to
!mr->readonly && !(mr->rom_device && mr->readable) or by applying
twice, !(mr->readonly || mr->rom_device && mr->readable), neither of
which makes much sense.
> mr->name,
> mr->alias->name,
> mr->alias_offset,
> mr->alias_offset
> + (target_phys_addr_t)int128_get64(mr->size) - 1);
> } else {
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
> + mon_printf(f,
> + TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readable ? 'R' : '-',
> + !mr->readonly && (!mr->rom_device || !mr->readable) ? 'W'
> + : '-',
> mr->name);
> }
>
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v3] memory-region: Report if region is read-only or write-only on info mtree
2012-02-04 15:04 ` Blue Swirl
@ 2012-02-04 15:25 ` Jan Kiszka
2012-02-11 11:09 ` Blue Swirl
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2012-02-04 15:25 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, Avi Kivity
From: Jan Kiszka <jan.kiszka@siemens.com>
Helpful to understand guest configurations of things like the i440FX's
PAM or the state of ROM devices.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes in v3:
- refactored writable condition (which DO makes sense as a ROM device
is either in ROM or in write mode. It's readable and writable in
both modes, but only in special ways. So let's tag it abstractly.)
memory.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index 5e77d8a..22816e2 100644
--- a/memory.c
+++ b/memory.c
@@ -1609,23 +1609,31 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
ml->printed = false;
QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
}
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
- TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
+ mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
+ " (prio %d, %c%c): alias %s @%s " TARGET_FMT_plx
+ "-" TARGET_FMT_plx "\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readable ? 'R' : '-',
+ !mr->readonly && !(mr->rom_device && mr->readable) ? 'W'
+ : '-',
mr->name,
mr->alias->name,
mr->alias_offset,
mr->alias_offset
+ (target_phys_addr_t)int128_get64(mr->size) - 1);
} else {
- mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
+ mon_printf(f,
+ TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
base + mr->addr,
base + mr->addr
+ (target_phys_addr_t)int128_get64(mr->size) - 1,
mr->priority,
+ mr->readable ? 'R' : '-',
+ !mr->readonly && !(mr->rom_device && mr->readable) ? 'W'
+ : '-',
mr->name);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-04 14:51 ` Jan Kiszka
@ 2012-02-09 8:35 ` Stefan Hajnoczi
2012-02-09 10:08 ` Jan Kiszka
0 siblings, 1 reply; 12+ messages in thread
From: Stefan Hajnoczi @ 2012-02-09 8:35 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Blue Swirl, qemu-trivial, qemu-devel, Avi Kivity
On Sat, Feb 04, 2012 at 03:51:43PM +0100, Jan Kiszka wrote:
> On 2012-02-04 13:32, Blue Swirl wrote:
> > On Sat, Feb 4, 2012 at 12:23, Jan Kiszka <jan.kiszka@web.de> wrote:
> >> On 2012-02-04 13:12, Blue Swirl wrote:
> >>> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>> Helpful to understand guest configurations of things like the i440FX's
> >>>> PAM.
> >>>>
> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>> ---
> >>>> memory.c | 6 ++++--
> >>>> 1 files changed, 4 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/memory.c b/memory.c
> >>>> index ee4c98a..ea4adda 100644
> >>>> --- a/memory.c
> >>>> +++ b/memory.c
> >>>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
> >>>> ml->printed = false;
> >>>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
> >>>> }
> >>>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
> >>>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
> >>>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
> >>>> base + mr->addr,
> >>>> base + mr->addr
> >>>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> >>>> mr->priority,
> >>>> + mr->readonly ? "RO" : "RW",
> >>>
> >>> I think the reserved regions which are unreadable and unwritable
> >>> should be shown as well. Then the output should be a combination of
> >>> 'R', 'W' or neither ('-').
> >>
> >> Reserved regions are in the hand of some other device model (so far only
> >> the KVM kernel). That says nothing about their R/W property. If we ever
> >> have a reserved region that is not writable, the owner could still set
> >> the corresponding flag for documentation purposes.
> >
> > OK. But it's also possible for a region to have readable == false
> > while readonly == false, which would imply 'WO' or '-W'. That also
> > supports separate 'R', 'W' and '-' flags.
>
> Yep, I encoded the ROM device state as well. And this revealed a
> regression of the memory region conversion of the cfi02. Gave up
> counting how often I fixed this type of bug in the flash code.
Is this patch ready to go? Seems fine to me but do you still want to
add the 'WO' output that Blue Swirl suggested?
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-09 8:35 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
@ 2012-02-09 10:08 ` Jan Kiszka
2012-02-09 11:54 ` Stefan Hajnoczi
0 siblings, 1 reply; 12+ messages in thread
From: Jan Kiszka @ 2012-02-09 10:08 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Blue Swirl, qemu-trivial, qemu-devel, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]
On 2012-02-09 09:35, Stefan Hajnoczi wrote:
> On Sat, Feb 04, 2012 at 03:51:43PM +0100, Jan Kiszka wrote:
>> On 2012-02-04 13:32, Blue Swirl wrote:
>>> On Sat, Feb 4, 2012 at 12:23, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>> On 2012-02-04 13:12, Blue Swirl wrote:
>>>>> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>> Helpful to understand guest configurations of things like the i440FX's
>>>>>> PAM.
>>>>>>
>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> ---
>>>>>> memory.c | 6 ++++--
>>>>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/memory.c b/memory.c
>>>>>> index ee4c98a..ea4adda 100644
>>>>>> --- a/memory.c
>>>>>> +++ b/memory.c
>>>>>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
>>>>>> ml->printed = false;
>>>>>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>>>>>> }
>>>>>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
>>>>>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
>>>>>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
>>>>>> base + mr->addr,
>>>>>> base + mr->addr
>>>>>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
>>>>>> mr->priority,
>>>>>> + mr->readonly ? "RO" : "RW",
>>>>>
>>>>> I think the reserved regions which are unreadable and unwritable
>>>>> should be shown as well. Then the output should be a combination of
>>>>> 'R', 'W' or neither ('-').
>>>>
>>>> Reserved regions are in the hand of some other device model (so far only
>>>> the KVM kernel). That says nothing about their R/W property. If we ever
>>>> have a reserved region that is not writable, the owner could still set
>>>> the corresponding flag for documentation purposes.
>>>
>>> OK. But it's also possible for a region to have readable == false
>>> while readonly == false, which would imply 'WO' or '-W'. That also
>>> supports separate 'R', 'W' and '-' flags.
>>
>> Yep, I encoded the ROM device state as well. And this revealed a
>> regression of the memory region conversion of the cfi02. Gave up
>> counting how often I fixed this type of bug in the flash code.
>
> Is this patch ready to go? Seems fine to me but do you still want to
> add the 'WO' output that Blue Swirl suggested?
There is v2 on the list, but I removed trival from CC due to the
discussion. Needs an ack from Blue, I think.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [Qemu-trivial] [PATCH] memory-region: Report if region is read-only on info mtree
2012-02-09 10:08 ` Jan Kiszka
@ 2012-02-09 11:54 ` Stefan Hajnoczi
0 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2012-02-09 11:54 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Blue Swirl, qemu-trivial, qemu-devel, Avi Kivity
On Thu, Feb 9, 2012 at 10:08 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> On 2012-02-09 09:35, Stefan Hajnoczi wrote:
>> On Sat, Feb 04, 2012 at 03:51:43PM +0100, Jan Kiszka wrote:
>>> On 2012-02-04 13:32, Blue Swirl wrote:
>>>> On Sat, Feb 4, 2012 at 12:23, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>> On 2012-02-04 13:12, Blue Swirl wrote:
>>>>>> On Fri, Feb 3, 2012 at 12:02, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>> Helpful to understand guest configurations of things like the i440FX's
>>>>>>> PAM.
>>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>> ---
>>>>>>> memory.c | 6 ++++--
>>>>>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/memory.c b/memory.c
>>>>>>> index ee4c98a..ea4adda 100644
>>>>>>> --- a/memory.c
>>>>>>> +++ b/memory.c
>>>>>>> @@ -1608,23 +1608,25 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
>>>>>>> ml->printed = false;
>>>>>>> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
>>>>>>> }
>>>>>>> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
>>>>>>> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s "
>>>>>>> TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
>>>>>>> base + mr->addr,
>>>>>>> base + mr->addr
>>>>>>> + (target_phys_addr_t)int128_get64(mr->size) - 1,
>>>>>>> mr->priority,
>>>>>>> + mr->readonly ? "RO" : "RW",
>>>>>>
>>>>>> I think the reserved regions which are unreadable and unwritable
>>>>>> should be shown as well. Then the output should be a combination of
>>>>>> 'R', 'W' or neither ('-').
>>>>>
>>>>> Reserved regions are in the hand of some other device model (so far only
>>>>> the KVM kernel). That says nothing about their R/W property. If we ever
>>>>> have a reserved region that is not writable, the owner could still set
>>>>> the corresponding flag for documentation purposes.
>>>>
>>>> OK. But it's also possible for a region to have readable == false
>>>> while readonly == false, which would imply 'WO' or '-W'. That also
>>>> supports separate 'R', 'W' and '-' flags.
>>>
>>> Yep, I encoded the ROM device state as well. And this revealed a
>>> regression of the memory region conversion of the cfi02. Gave up
>>> counting how often I fixed this type of bug in the flash code.
>>
>> Is this patch ready to go? Seems fine to me but do you still want to
>> add the 'WO' output that Blue Swirl suggested?
>
> There is v2 on the list, but I removed trival from CC due to the
> discussion. Needs an ack from Blue, I think.
Okay, I missed it. Thanks for the update!
Stefan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v3] memory-region: Report if region is read-only or write-only on info mtree
2012-02-04 15:25 ` [Qemu-devel] [PATCH v3] " Jan Kiszka
@ 2012-02-11 11:09 ` Blue Swirl
0 siblings, 0 replies; 12+ messages in thread
From: Blue Swirl @ 2012-02-11 11:09 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Avi Kivity
Thanks, applied.
On Sat, Feb 4, 2012 at 15:25, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Helpful to understand guest configurations of things like the i440FX's
> PAM or the state of ROM devices.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v3:
> - refactored writable condition (which DO makes sense as a ROM device
> is either in ROM or in write mode. It's readable and writable in
> both modes, but only in special ways. So let's tag it abstractly.)
>
> memory.c | 14 +++++++++++---
> 1 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/memory.c b/memory.c
> index 5e77d8a..22816e2 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -1609,23 +1609,31 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
> ml->printed = false;
> QTAILQ_INSERT_TAIL(alias_print_queue, ml, queue);
> }
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
> - TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
> + mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx
> + " (prio %d, %c%c): alias %s @%s " TARGET_FMT_plx
> + "-" TARGET_FMT_plx "\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readable ? 'R' : '-',
> + !mr->readonly && !(mr->rom_device && mr->readable) ? 'W'
> + : '-',
> mr->name,
> mr->alias->name,
> mr->alias_offset,
> mr->alias_offset
> + (target_phys_addr_t)int128_get64(mr->size) - 1);
> } else {
> - mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
> + mon_printf(f,
> + TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s\n",
> base + mr->addr,
> base + mr->addr
> + (target_phys_addr_t)int128_get64(mr->size) - 1,
> mr->priority,
> + mr->readable ? 'R' : '-',
> + !mr->readonly && !(mr->rom_device && mr->readable) ? 'W'
> + : '-',
> mr->name);
> }
>
> --
> 1.7.3.4
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-02-11 11:09 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-03 12:02 [Qemu-devel] [PATCH] memory-region: Report if region is read-only on info mtree Jan Kiszka
2012-02-04 12:12 ` Blue Swirl
2012-02-04 12:23 ` Jan Kiszka
2012-02-04 12:32 ` Blue Swirl
2012-02-04 14:51 ` Jan Kiszka
2012-02-09 8:35 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
2012-02-09 10:08 ` Jan Kiszka
2012-02-09 11:54 ` Stefan Hajnoczi
2012-02-04 14:57 ` [Qemu-devel] [PATCH v2] memory-region: Report if region is read-only or write-only " Jan Kiszka
2012-02-04 15:04 ` Blue Swirl
2012-02-04 15:25 ` [Qemu-devel] [PATCH v3] " Jan Kiszka
2012-02-11 11:09 ` Blue Swirl
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).