* [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
@ 2011-09-18 12:44 Jan Kiszka
2011-09-18 15:51 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-09-18 12:44 UTC (permalink / raw)
To: Avi Kivity; +Cc: qemu-devel, Richard Henderson
From: Jan Kiszka <jan.kiszka@siemens.com>
Fix copy&paste errors and reduce duplications of the BOCHS VBE ranges.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
This and the previous patch unbreaks VGA over memory/master.
hw/vga.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/vga.c b/hw/vga.c
index 513a5f6..22160e8 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2244,19 +2244,18 @@ static const MemoryRegionPortio vga_portio_list[] = {
{ 0x04, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3b4 */
{ 0x0a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3ba */
{ 0x10, 16, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3c0 */
- { 0x14, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3d4 */
- { 0x1a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3da */
+ { 0x24, 2, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3d4 */
+ { 0x2a, 1, 1, .read = vga_ioport_read, .write = vga_ioport_write }, /* 3da */
PORTIO_END_OF_LIST(),
};
#ifdef CONFIG_BOCHS_VBE
static const MemoryRegionPortio vbe_portio_list[] = {
-# ifdef TARGET_I386
{ 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
+# ifdef TARGET_I386
{ 1, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
# else
- { 0, 2, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
- { 2, 2, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
+ { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
# endif
PORTIO_END_OF_LIST(),
};
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
2011-09-18 12:44 [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts Jan Kiszka
@ 2011-09-18 15:51 ` Avi Kivity
2011-09-18 16:28 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2011-09-18 15:51 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Richard Henderson
On 09/18/2011 03:44 PM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> Fix copy&paste errors and reduce duplications of the BOCHS VBE ranges.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>
> #ifdef CONFIG_BOCHS_VBE
> static const MemoryRegionPortio vbe_portio_list[] = {
> -# ifdef TARGET_I386
> { 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
> +# ifdef TARGET_I386
Strange how the card knows what target it's plugged into. Fixing this
is going to involve major pain - need to update the bios and need to
provide backwards compatibility. All this assuming guests don't
hardcode the address.
Anyway I find the deduplication makes the code harder to read.
> { 1, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
> # else
> - { 0, 2, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index },
> - { 2, 2, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
> + { 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
Is this correct? length 1 region with size 2?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
2011-09-18 15:51 ` Avi Kivity
@ 2011-09-18 16:28 ` Jan Kiszka
2011-09-18 16:51 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-09-18 16:28 UTC (permalink / raw)
To: Avi Kivity; +Cc: qemu-devel, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]
On 2011-09-18 17:51, Avi Kivity wrote:
> On 09/18/2011 03:44 PM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> Fix copy&paste errors and reduce duplications of the BOCHS VBE ranges.
>>
>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>> ---
>>
>> #ifdef CONFIG_BOCHS_VBE
>> static const MemoryRegionPortio vbe_portio_list[] = {
>> -# ifdef TARGET_I386
>> { 0, 1, 2, .read = vbe_ioport_read_index, .write =
>> vbe_ioport_write_index },
>> +# ifdef TARGET_I386
>
> Strange how the card knows what target it's plugged into. Fixing this
> is going to involve major pain - need to update the bios and need to
> provide backwards compatibility. All this assuming guests don't
> hardcode the address.
>
> Anyway I find the deduplication makes the code harder to read.
That line is rather tricky to deduplicate in mind. So it's not nice for
our readers keeping it duplicated around.
>
>> { 1, 1, 2, .read = vbe_ioport_read_data, .write =
>> vbe_ioport_write_data },
>> # else
>> - { 0, 2, 2, .read = vbe_ioport_read_index, .write =
>> vbe_ioport_write_index },
>> - { 2, 2, 2, .read = vbe_ioport_read_data, .write =
>> vbe_ioport_write_data },
>> + { 2, 1, 2, .read = vbe_ioport_read_data, .write =
>> vbe_ioport_write_data },
>
> Is this correct? length 1 region with size 2?
Yes, it prevents accesses on base+1.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
2011-09-18 16:28 ` Jan Kiszka
@ 2011-09-18 16:51 ` Avi Kivity
2011-09-18 19:07 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2011-09-18 16:51 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Richard Henderson
On 09/18/2011 07:28 PM, Jan Kiszka wrote:
> On 2011-09-18 17:51, Avi Kivity wrote:
> > On 09/18/2011 03:44 PM, Jan Kiszka wrote:
> >> From: Jan Kiszka<jan.kiszka@siemens.com>
> >>
> >> Fix copy&paste errors and reduce duplications of the BOCHS VBE ranges.
> >>
> >> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> >> ---
> >>
> >> #ifdef CONFIG_BOCHS_VBE
> >> static const MemoryRegionPortio vbe_portio_list[] = {
> >> -# ifdef TARGET_I386
> >> { 0, 1, 2, .read = vbe_ioport_read_index, .write =
> >> vbe_ioport_write_index },
> >> +# ifdef TARGET_I386
> >
> > Strange how the card knows what target it's plugged into. Fixing this
> > is going to involve major pain - need to update the bios and need to
> > provide backwards compatibility. All this assuming guests don't
> > hardcode the address.
> >
> > Anyway I find the deduplication makes the code harder to read.
>
> That line is rather tricky to deduplicate in mind. So it's not nice for
> our readers keeping it duplicated around.
I don't think it should be deduplicated. The device is providing two
separate ABIs.
> >
> >> { 1, 1, 2, .read = vbe_ioport_read_data, .write =
> >> vbe_ioport_write_data },
> >> # else
> >> - { 0, 2, 2, .read = vbe_ioport_read_index, .write =
> >> vbe_ioport_write_index },
> >> - { 2, 2, 2, .read = vbe_ioport_read_data, .write =
> >> vbe_ioport_write_data },
> >> + { 2, 1, 2, .read = vbe_ioport_read_data, .write =
> >> vbe_ioport_write_data },
> >
> > Is this correct? length 1 region with size 2?
>
> Yes, it prevents accesses on base+1.
>
Why disallow it?
For the general case or a 2-byte access near the end of a region, we
need to provide a general fix.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
2011-09-18 16:51 ` Avi Kivity
@ 2011-09-18 19:07 ` Jan Kiszka
2011-09-19 12:22 ` Avi Kivity
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2011-09-18 19:07 UTC (permalink / raw)
To: Avi Kivity; +Cc: qemu-devel, Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]
On 2011-09-18 18:51, Avi Kivity wrote:
> On 09/18/2011 07:28 PM, Jan Kiszka wrote:
>> On 2011-09-18 17:51, Avi Kivity wrote:
>> > On 09/18/2011 03:44 PM, Jan Kiszka wrote:
>> >> From: Jan Kiszka<jan.kiszka@siemens.com>
>> >>
>> >> Fix copy&paste errors and reduce duplications of the BOCHS VBE
>> ranges.
>> >>
>> >> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>> >> ---
>> >>
>> >> #ifdef CONFIG_BOCHS_VBE
>> >> static const MemoryRegionPortio vbe_portio_list[] = {
>> >> -# ifdef TARGET_I386
>> >> { 0, 1, 2, .read = vbe_ioport_read_index, .write =
>> >> vbe_ioport_write_index },
>> >> +# ifdef TARGET_I386
>> >
>> > Strange how the card knows what target it's plugged into. Fixing this
>> > is going to involve major pain - need to update the bios and need to
>> > provide backwards compatibility. All this assuming guests don't
>> > hardcode the address.
>> >
>> > Anyway I find the deduplication makes the code harder to read.
>>
>> That line is rather tricky to deduplicate in mind. So it's not nice for
>> our readers keeping it duplicated around.
>
> I don't think it should be deduplicated. The device is providing two
> separate ABIs.
Yes, two ABIs, and the only difference is the offset of the data register.
>
>> >
>> >> { 1, 1, 2, .read = vbe_ioport_read_data, .write =
>> >> vbe_ioport_write_data },
>> >> # else
>> >> - { 0, 2, 2, .read = vbe_ioport_read_index, .write =
>> >> vbe_ioport_write_index },
>> >> - { 2, 2, 2, .read = vbe_ioport_read_data, .write =
>> >> vbe_ioport_write_data },
>> >> + { 2, 1, 2, .read = vbe_ioport_read_data, .write =
>> >> vbe_ioport_write_data },
>> >
>> > Is this correct? length 1 region with size 2?
>>
>> Yes, it prevents accesses on base+1.
>>
>
> Why disallow it?
Did anyone check that something useful or at least valid comes out of
the handlers when doing this so far impossible access?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts
2011-09-18 19:07 ` Jan Kiszka
@ 2011-09-19 12:22 ` Avi Kivity
0 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2011-09-19 12:22 UTC (permalink / raw)
To: Jan Kiszka; +Cc: qemu-devel, Richard Henderson
On 09/18/2011 10:07 PM, Jan Kiszka wrote:
> > I don't think it should be deduplicated. The device is providing two
> > separate ABIs.
>
> Yes, two ABIs, and the only difference is the offset of the data register.
How about
#ifdef TARGET_I386
# define VBE_DATA_REG 1
#else
# define VBE_DATA_REG 2
#endif
> >
> > Why disallow it?
>
> Did anyone check that something useful or at least valid comes out of
> the handlers when doing this so far impossible access?
>
It's a general problem. It can't be fixed in devices, the core has to
handle this (and devices have to tell it how to react to such accesses).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-19 12:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-18 12:44 [Qemu-devel] [PATCH 2/2] vga: Fix portio list conversion fallouts Jan Kiszka
2011-09-18 15:51 ` Avi Kivity
2011-09-18 16:28 ` Jan Kiszka
2011-09-18 16:51 ` Avi Kivity
2011-09-18 19:07 ` Jan Kiszka
2011-09-19 12:22 ` Avi Kivity
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).