qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] start qemu failed with --enable-kvm -vga std
@ 2009-04-08  3:11 Peng Huang
  2009-04-08 12:27 ` Paul Brook
  2009-04-08 13:12 ` Anthony Liguori
  0 siblings, 2 replies; 20+ messages in thread
From: Peng Huang @ 2009-04-08  3:11 UTC (permalink / raw)
  To: qemu-devel

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

Hi,

The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
-vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
call cpu_register_physical_memory with a wrong size. Below patch can fix it
on my box. Please test it.

Regards,
Peng Huang


diff --git a/hw/pc.c b/hw/pc.c
index f9cfd1f..7775c7b 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -876,7 +876,7 @@ vga_bios_error:
             exit(1);
         }
        /* Round up vga bios size to the next 2k boundary */
-       vga_bios_size = (vga_bios_size + 2047) & ~2047;
+       vga_bios_size = (vga_bios_size + 4095) & ~4095;
        option_rom_start = 0xc0000 + vga_bios_size;

         /* setup basic memory access */

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] start qemu failed with --enable-kvm -vga std
  2009-04-08  3:11 [Qemu-devel] start qemu failed with --enable-kvm -vga std Peng Huang
@ 2009-04-08 12:27 ` Paul Brook
  2009-04-08 13:12 ` Anthony Liguori
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Brook @ 2009-04-08 12:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peng Huang

On Wednesday 08 April 2009, Peng Huang wrote:
> Hi,
>
> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of
> qemu call cpu_register_physical_memory with a wrong size. Below patch can
> fix it on my box. Please test it.

This is almost certainly the wrong way to fix this.

Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] start qemu failed with --enable-kvm -vga std
  2009-04-08  3:11 [Qemu-devel] start qemu failed with --enable-kvm -vga std Peng Huang
  2009-04-08 12:27 ` Paul Brook
@ 2009-04-08 13:12 ` Anthony Liguori
  2009-04-08 13:25   ` Glauber Costa
  1 sibling, 1 reply; 20+ messages in thread
From: Anthony Liguori @ 2009-04-08 13:12 UTC (permalink / raw)
  To: qemu-devel, Glauber Costa

Peng Huang wrote:
> Hi,
>
> The HEAD version qemu can not execute a VM with --enable-kvm -vga std 
> or -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is 
> because of qemu call cpu_register_physical_memory with a wrong size. 
> Below patch can fix it on my box. Please test it.

Glauber came up with a similar patch for kvm-userspace and is currently 
attempting to root cause the issue.

Regards,

Anthony Liguori

> Regards,
> Peng Huang
>  
>
> diff --git a/hw/pc.c b/hw/pc.c
> index f9cfd1f..7775c7b 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -876,7 +876,7 @@ vga_bios_error:
>              exit(1);
>          }
>         /* Round up vga bios size to the next 2k boundary */
> -       vga_bios_size = (vga_bios_size + 2047) & ~2047;
> +       vga_bios_size = (vga_bios_size + 4095) & ~4095;
>         option_rom_start = 0xc0000 + vga_bios_size;
>  
>          /* setup basic memory access */
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] start qemu failed with --enable-kvm -vga std
  2009-04-08 13:12 ` Anthony Liguori
@ 2009-04-08 13:25   ` Glauber Costa
  2009-04-08 16:34     ` [Qemu-devel] " Jan Kiszka
  0 siblings, 1 reply; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 13:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Peng Huang wrote:
>>
>> Hi,
>>
>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>> on my box. Please test it.
>
> Glauber came up with a similar patch for kvm-userspace and is currently
> attempting to root cause the issue.

I believe this is in fact the root cause. KVM slot management code
probably require
memory to be page aligned. By trying to register a region that is not
page aligned,
the ioctl may fail. I, however, did not see this happening in qemu
upstream (only kvm-userspace),
and have absolutely no idea about why. But the patch makes perfect sense to me.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 13:25   ` Glauber Costa
@ 2009-04-08 16:34     ` Jan Kiszka
  2009-04-08 16:41       ` Glauber Costa
  2009-04-08 17:31       ` Paul Brook
  0 siblings, 2 replies; 20+ messages in thread
From: Jan Kiszka @ 2009-04-08 16:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

Glauber Costa wrote:
> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>> Peng Huang wrote:
>>> Hi,
>>>
>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>> on my box. Please test it.
>> Glauber came up with a similar patch for kvm-userspace and is currently
>> attempting to root cause the issue.
> 
> I believe this is in fact the root cause. KVM slot management code
> probably require
> memory to be page aligned. By trying to register a region that is not
> page aligned,
> the ioctl may fail. I, however, did not see this happening in qemu
> upstream (only kvm-userspace),
> and have absolutely no idea about why. But the patch makes perfect sense to me.
> 

But this really sounds like a limitation that should better be fixed in
the kvm layer, not the device/machine code.

We only map ROM regions here. So rounding them up/down and sending
properly aligned requests to the kernel should finally have the same
result for all involved pages. Only if non-compatible regions overlap
due to such roundings, we should bail out - and start to consider
changing device code.

My next free time slot is reserved now for the anyway required
enhancement of kvm's slot management in qemu. kvm-userspace could catch
up afterwards (finally fixing its broken text console after reset...).

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 16:34     ` [Qemu-devel] " Jan Kiszka
@ 2009-04-08 16:41       ` Glauber Costa
  2009-04-08 17:02         ` Jan Kiszka
  2009-04-08 17:31       ` Paul Brook
  1 sibling, 1 reply; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Glauber Costa wrote:
>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>> Peng Huang wrote:
>>>> Hi,
>>>>
>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>> on my box. Please test it.
>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>> attempting to root cause the issue.
>>
>> I believe this is in fact the root cause. KVM slot management code
>> probably require
>> memory to be page aligned. By trying to register a region that is not
>> page aligned,
>> the ioctl may fail. I, however, did not see this happening in qemu
>> upstream (only kvm-userspace),
>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>
>
> But this really sounds like a limitation that should better be fixed in
> the kvm layer, not the device/machine code.
>
> We only map ROM regions here. So rounding them up/down and sending
> properly aligned requests to the kernel should finally have the same
> result for all involved pages. Only if non-compatible regions overlap
> due to such roundings, we should bail out - and start to consider
> changing device code.

I've considered this, but then the alignment constraints become implicit,
rather than explicit. Consider the option rom registration code itself.

We should start loading option roms right after vga. If kvm layer
aligns the region
implicitly, then where will option roms start? At better, we would have to tell
qemu back that such an alignment were made, to start looking for option
roms in the right place.

I believe being explicit, and requiring page alignment is safer, and
does not hurt.

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 16:41       ` Glauber Costa
@ 2009-04-08 17:02         ` Jan Kiszka
  2009-04-08 17:06           ` Glauber Costa
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Kiszka @ 2009-04-08 17:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

Glauber Costa wrote:
> On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Glauber Costa wrote:
>>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>>> Peng Huang wrote:
>>>>> Hi,
>>>>>
>>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>>> on my box. Please test it.
>>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>>> attempting to root cause the issue.
>>> I believe this is in fact the root cause. KVM slot management code
>>> probably require
>>> memory to be page aligned. By trying to register a region that is not
>>> page aligned,
>>> the ioctl may fail. I, however, did not see this happening in qemu
>>> upstream (only kvm-userspace),
>>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>>
>> But this really sounds like a limitation that should better be fixed in
>> the kvm layer, not the device/machine code.
>>
>> We only map ROM regions here. So rounding them up/down and sending
>> properly aligned requests to the kernel should finally have the same
>> result for all involved pages. Only if non-compatible regions overlap
>> due to such roundings, we should bail out - and start to consider
>> changing device code.
> 
> I've considered this, but then the alignment constraints become implicit,
> rather than explicit. Consider the option rom registration code itself.
> 
> We should start loading option roms right after vga. If kvm layer

Unless I misread something, this is about option ROMs after VGA ROM.

> aligns the region
> implicitly, then where will option roms start? At better, we would have to tell
> qemu back that such an alignment were made, to start looking for option
> roms in the right place.
> 

My ideas is to merge both ROM regions (given we are actually talking
about compatible stuff here): the 2k VGA ROM would first be expanded to
4k, then the next option ROM starting after the VGA would extend the
latter. But that's plain theory so far.

> I believe being explicit, and requiring page alignment is safer, and
> does not hurt.

In this case, we would waste 2k of (sometimes) precious low mem...

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:02         ` Jan Kiszka
@ 2009-04-08 17:06           ` Glauber Costa
  2009-04-08 17:20             ` Jan Kiszka
  0 siblings, 1 reply; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 17:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

On Wed, Apr 8, 2009 at 2:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Glauber Costa wrote:
>> On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Glauber Costa wrote:
>>>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>>>> Peng Huang wrote:
>>>>>> Hi,
>>>>>>
>>>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>>>> on my box. Please test it.
>>>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>>>> attempting to root cause the issue.
>>>> I believe this is in fact the root cause. KVM slot management code
>>>> probably require
>>>> memory to be page aligned. By trying to register a region that is not
>>>> page aligned,
>>>> the ioctl may fail. I, however, did not see this happening in qemu
>>>> upstream (only kvm-userspace),
>>>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>>>
>>> But this really sounds like a limitation that should better be fixed in
>>> the kvm layer, not the device/machine code.
>>>
>>> We only map ROM regions here. So rounding them up/down and sending
>>> properly aligned requests to the kernel should finally have the same
>>> result for all involved pages. Only if non-compatible regions overlap
>>> due to such roundings, we should bail out - and start to consider
>>> changing device code.
>>
>> I've considered this, but then the alignment constraints become implicit,
>> rather than explicit. Consider the option rom registration code itself.
>>
>> We should start loading option roms right after vga. If kvm layer
>
> Unless I misread something, this is about option ROMs after VGA ROM.
>
>> aligns the region
>> implicitly, then where will option roms start? At better, we would have to tell
>> qemu back that such an alignment were made, to start looking for option
>> roms in the right place.
>>
>
> My ideas is to merge both ROM regions (given we are actually talking
> about compatible stuff here): the 2k VGA ROM would first be expanded to
> 4k, then the next option ROM starting after the VGA would extend the
> latter. But that's plain theory so far.
>
>> I believe being explicit, and requiring page alignment is safer, and
>> does not hurt.
>
> In this case, we would waste 2k of (sometimes) precious low mem...
>

If we can merge the regions, then I agree with you. We'll probably have an
alignment requirement anyway, but it can well be handled in kvm layer.

However, as you ack yourself, it is likely to take some time. In the mean time,
I believe this fix is acceptable. Unless you're planning on having something
to fix it in the next few days.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:06           ` Glauber Costa
@ 2009-04-08 17:20             ` Jan Kiszka
  2009-04-08 17:23               ` Glauber Costa
                                 ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Jan Kiszka @ 2009-04-08 17:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

Glauber Costa wrote:
> On Wed, Apr 8, 2009 at 2:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> Glauber Costa wrote:
>>> On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> Glauber Costa wrote:
>>>>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>>>>> Peng Huang wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>>>>> on my box. Please test it.
>>>>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>>>>> attempting to root cause the issue.
>>>>> I believe this is in fact the root cause. KVM slot management code
>>>>> probably require
>>>>> memory to be page aligned. By trying to register a region that is not
>>>>> page aligned,
>>>>> the ioctl may fail. I, however, did not see this happening in qemu
>>>>> upstream (only kvm-userspace),
>>>>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>>>>
>>>> But this really sounds like a limitation that should better be fixed in
>>>> the kvm layer, not the device/machine code.
>>>>
>>>> We only map ROM regions here. So rounding them up/down and sending
>>>> properly aligned requests to the kernel should finally have the same
>>>> result for all involved pages. Only if non-compatible regions overlap
>>>> due to such roundings, we should bail out - and start to consider
>>>> changing device code.
>>> I've considered this, but then the alignment constraints become implicit,
>>> rather than explicit. Consider the option rom registration code itself.
>>>
>>> We should start loading option roms right after vga. If kvm layer
>> Unless I misread something, this is about option ROMs after VGA ROM.
>>
>>> aligns the region
>>> implicitly, then where will option roms start? At better, we would have to tell
>>> qemu back that such an alignment were made, to start looking for option
>>> roms in the right place.
>>>
>> My ideas is to merge both ROM regions (given we are actually talking
>> about compatible stuff here): the 2k VGA ROM would first be expanded to
>> 4k, then the next option ROM starting after the VGA would extend the
>> latter. But that's plain theory so far.
>>
>>> I believe being explicit, and requiring page alignment is safer, and
>>> does not hurt.
>> In this case, we would waste 2k of (sometimes) precious low mem...
>>
> 
> If we can merge the regions, then I agree with you. We'll probably have an
> alignment requirement anyway, but it can well be handled in kvm layer.
> 
> However, as you ack yourself, it is likely to take some time. In the mean time,
> I believe this fix is acceptable. Unless you're planning on having something
> to fix it in the next few days.

Easter is coming - if I find my eggs quickly, there might be some time
left... ;)

I leave the decision up to the maintainers if we should merge this
workaround first, or wait a bit longer until we know for sure if we can
fix it in a cleaner way.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:20             ` Jan Kiszka
@ 2009-04-08 17:23               ` Glauber Costa
  2009-04-08 17:25               ` Glauber Costa
  2009-04-08 18:57               ` Anthony Liguori
  2 siblings, 0 replies; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 17:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

On Wed, Apr 8, 2009 at 2:20 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Glauber Costa wrote:
>> On Wed, Apr 8, 2009 at 2:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Glauber Costa wrote:
>>>> On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> Glauber Costa wrote:
>>>>>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>>>>>> Peng Huang wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>>>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>>>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>>>>>> on my box. Please test it.
>>>>>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>>>>>> attempting to root cause the issue.
>>>>>> I believe this is in fact the root cause. KVM slot management code
>>>>>> probably require
>>>>>> memory to be page aligned. By trying to register a region that is not
>>>>>> page aligned,
>>>>>> the ioctl may fail. I, however, did not see this happening in qemu
>>>>>> upstream (only kvm-userspace),
>>>>>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>>>>>
>>>>> But this really sounds like a limitation that should better be fixed in
>>>>> the kvm layer, not the device/machine code.
>>>>>
>>>>> We only map ROM regions here. So rounding them up/down and sending
>>>>> properly aligned requests to the kernel should finally have the same
>>>>> result for all involved pages. Only if non-compatible regions overlap
>>>>> due to such roundings, we should bail out - and start to consider
>>>>> changing device code.
>>>> I've considered this, but then the alignment constraints become implicit,
>>>> rather than explicit. Consider the option rom registration code itself.
>>>>
>>>> We should start loading option roms right after vga. If kvm layer
>>> Unless I misread something, this is about option ROMs after VGA ROM.
>>>
>>>> aligns the region
>>>> implicitly, then where will option roms start? At better, we would have to tell
>>>> qemu back that such an alignment were made, to start looking for option
>>>> roms in the right place.
>>>>
>>> My ideas is to merge both ROM regions (given we are actually talking
>>> about compatible stuff here): the 2k VGA ROM would first be expanded to
>>> 4k, then the next option ROM starting after the VGA would extend the
>>> latter. But that's plain theory so far.
>>>
>>>> I believe being explicit, and requiring page alignment is safer, and
>>>> does not hurt.
>>> In this case, we would waste 2k of (sometimes) precious low mem...
>>>
>>
>> If we can merge the regions, then I agree with you. We'll probably have an
>> alignment requirement anyway, but it can well be handled in kvm layer.
>>
>> However, as you ack yourself, it is likely to take some time. In the mean time,
>> I believe this fix is acceptable. Unless you're planning on having something
>> to fix it in the next few days.
>
> Easter is coming - if I find my eggs quickly, there might be some time
> left... ;)

You're not planning to use this opportunity to fill qemu code with
easter eggs, are you? ;-)


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:20             ` Jan Kiszka
  2009-04-08 17:23               ` Glauber Costa
@ 2009-04-08 17:25               ` Glauber Costa
  2009-04-08 17:39                 ` M. Warner Losh
  2009-04-08 18:58                 ` Anthony Liguori
  2009-04-08 18:57               ` Anthony Liguori
  2 siblings, 2 replies; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 17:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

On Wed, Apr 8, 2009 at 2:20 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Glauber Costa wrote:
>> On Wed, Apr 8, 2009 at 2:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> Glauber Costa wrote:
>>>> On Wed, Apr 8, 2009 at 1:34 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>> Glauber Costa wrote:
>>>>>> On Wed, Apr 8, 2009 at 10:12 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
>>>>>>> Peng Huang wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> The HEAD version qemu can not execute a VM with --enable-kvm -vga std or
>>>>>>>> -vga vmware on kernel 2.6.29.1-46.fc11.x86_64. I found it is because of qemu
>>>>>>>> call cpu_register_physical_memory with a wrong size. Below patch can fix it
>>>>>>>> on my box. Please test it.
>>>>>>> Glauber came up with a similar patch for kvm-userspace and is currently
>>>>>>> attempting to root cause the issue.
>>>>>> I believe this is in fact the root cause. KVM slot management code
>>>>>> probably require
>>>>>> memory to be page aligned. By trying to register a region that is not
>>>>>> page aligned,
>>>>>> the ioctl may fail. I, however, did not see this happening in qemu
>>>>>> upstream (only kvm-userspace),
>>>>>> and have absolutely no idea about why. But the patch makes perfect sense to me.
>>>>>>
>>>>> But this really sounds like a limitation that should better be fixed in
>>>>> the kvm layer, not the device/machine code.
>>>>>
>>>>> We only map ROM regions here. So rounding them up/down and sending
>>>>> properly aligned requests to the kernel should finally have the same
>>>>> result for all involved pages. Only if non-compatible regions overlap
>>>>> due to such roundings, we should bail out - and start to consider
>>>>> changing device code.
>>>> I've considered this, but then the alignment constraints become implicit,
>>>> rather than explicit. Consider the option rom registration code itself.
>>>>
>>>> We should start loading option roms right after vga. If kvm layer
>>> Unless I misread something, this is about option ROMs after VGA ROM.
>>>
>>>> aligns the region
>>>> implicitly, then where will option roms start? At better, we would have to tell
>>>> qemu back that such an alignment were made, to start looking for option
>>>> roms in the right place.
>>>>
>>> My ideas is to merge both ROM regions (given we are actually talking
>>> about compatible stuff here): the 2k VGA ROM would first be expanded to
>>> 4k, then the next option ROM starting after the VGA would extend the
>>> latter. But that's plain theory so far.
>>>
>>>> I believe being explicit, and requiring page alignment is safer, and
>>>> does not hurt.
>>> In this case, we would waste 2k of (sometimes) precious low mem...
>>>
>>
>> If we can merge the regions, then I agree with you. We'll probably have an
>> alignment requirement anyway, but it can well be handled in kvm layer.
>>
>> However, as you ack yourself, it is likely to take some time. In the mean time,
>> I believe this fix is acceptable. Unless you're planning on having something
>> to fix it in the next few days.
>
> Easter is coming - if I find my eggs quickly, there might be some time
> left... ;)
>
> I leave the decision up to the maintainers if we should merge this
> workaround first, or wait a bit longer until we know for sure if we can
> fix it in a cleaner way.
In all seriousness, no matter how good your series for improving kvm
registration
are, it is unlikely to hit the stable branch. So I believe a good deal
would have something
on the line of this patch applied to stable, leaving unstable as is,
until we can find a better
solution.

What do you think, anthony?


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 16:34     ` [Qemu-devel] " Jan Kiszka
  2009-04-08 16:41       ` Glauber Costa
@ 2009-04-08 17:31       ` Paul Brook
  1 sibling, 0 replies; 20+ messages in thread
From: Paul Brook @ 2009-04-08 17:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jan Kiszka, Glauber Costa

> > I believe this is in fact the root cause. KVM slot management code
> > probably require
> > memory to be page aligned. By trying to register a region that is not
> > page aligned,
> > the ioctl may fail. I, however, did not see this happening in qemu
> > upstream (only kvm-userspace),
> > and have absolutely no idea about why. But the patch makes perfect sense
> > to me.
>
> But this really sounds like a limitation that should better be fixed in
> the kvm layer, not the device/machine code.

I was meaning more why we're creating regions for individual roms. Just map 
the whole block at once.

FWIW kvm probably needs to be able to handle small regions anyway, some PCI 
devices require them.

Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:25               ` Glauber Costa
@ 2009-04-08 17:39                 ` M. Warner Losh
  2009-04-08 17:44                   ` Glauber Costa
  2009-04-08 17:47                   ` Paul Brook
  2009-04-08 18:58                 ` Anthony Liguori
  1 sibling, 2 replies; 20+ messages in thread
From: M. Warner Losh @ 2009-04-08 17:39 UTC (permalink / raw)
  To: qemu-devel, glommer; +Cc: glommer

In message: <5d6222a80904081025j1644b6bay17d1ff942247971@mail.gmail.com>
            Glauber Costa <glommer@gmail.com> writes:
: In all seriousness, no matter how good your series for improving kvm
: registration
: are, it is unlikely to hit the stable branch. So I believe a good deal
: would have something
: on the line of this patch applied to stable, leaving unstable as is,
: until we can find a better
: solution.

Huh?  It is unlikely to be merged into stable, so let's commit it to
stable?

Warner

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:39                 ` M. Warner Losh
@ 2009-04-08 17:44                   ` Glauber Costa
  2009-04-08 17:57                     ` M. Warner Losh
  2009-04-08 17:47                   ` Paul Brook
  1 sibling, 1 reply; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 17:44 UTC (permalink / raw)
  To: M. Warner Losh; +Cc: glommer, qemu-devel

On Wed, Apr 8, 2009 at 2:39 PM, M. Warner Losh <imp@bsdimp.com> wrote:
> In message: <5d6222a80904081025j1644b6bay17d1ff942247971@mail.gmail.com>
>            Glauber Costa <glommer@gmail.com> writes:
> : In all seriousness, no matter how good your series for improving kvm
> : registration
> : are, it is unlikely to hit the stable branch. So I believe a good deal
> : would have something
> : on the line of this patch applied to stable, leaving unstable as is,
> : until we can find a better
> : solution.
>
> Huh?  It is unlikely to be merged into stable, so let's commit it to
> stable?
Yes. It is a kohan, and whenever you understand it, you'll reach
enlightment. So I won't
explain that to you ;-)

I meant that Jan's forthcoming series that heavily touch kvm memory
registration (the beautiful way)
is unlikely to get into stable. So I'd advocate applying this patch
(the said ugly way) to stable.
Otherwise we'll get -vga std broken for good.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:39                 ` M. Warner Losh
  2009-04-08 17:44                   ` Glauber Costa
@ 2009-04-08 17:47                   ` Paul Brook
  2009-04-08 18:07                     ` Glauber Costa
  1 sibling, 1 reply; 20+ messages in thread
From: Paul Brook @ 2009-04-08 17:47 UTC (permalink / raw)
  To: qemu-devel; +Cc: glommer, glommer

On Wednesday 08 April 2009, M. Warner Losh wrote:
> In message: <5d6222a80904081025j1644b6bay17d1ff942247971@mail.gmail.com>
>
>             Glauber Costa <glommer@gmail.com> writes:
> : In all seriousness, no matter how good your series for improving kvm
> : registration
> : are, it is unlikely to hit the stable branch. So I believe a good deal
> : would have something
> : on the line of this patch applied to stable, leaving unstable as is,
> : until we can find a better
> : solution.
>
> Huh?  It is unlikely to be merged into stable, so let's commit it to
> stable?

I think the suggestion is that we apply an ugly hack on just the stable 
branch, and fix it properly later on the development.

FWIW I think this is a bad idea. We should never fix something on the stable 
branch that isn't also fixed on the development branch. If the changes are 
causing problems, then my suggestion is to revert the original change (i.e. 
the support for >64k roms).

Paul

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:44                   ` Glauber Costa
@ 2009-04-08 17:57                     ` M. Warner Losh
  0 siblings, 0 replies; 20+ messages in thread
From: M. Warner Losh @ 2009-04-08 17:57 UTC (permalink / raw)
  To: glommer; +Cc: glommer, qemu-devel

In message: <5d6222a80904081044u60ba9a4dqd6b833eb0738503@mail.gmail.com>
            Glauber Costa <glommer@gmail.com> writes:
: On Wed, Apr 8, 2009 at 2:39 PM, M. Warner Losh <imp@bsdimp.com> wrote:
: > In message: <5d6222a80904081025j1644b6bay17d1ff942247971@mail.gmail.com>
: >            Glauber Costa <glommer@gmail.com> writes:
: > : In all seriousness, no matter how good your series for improving kvm
: > : registration
: > : are, it is unlikely to hit the stable branch. So I believe a good deal
: > : would have something
: > : on the line of this patch applied to stable, leaving unstable as is,
: > : until we can find a better
: > : solution.
: >
: > Huh?  It is unlikely to be merged into stable, so let's commit it to
: > stable?
: Yes. It is a kohan, and whenever you understand it, you'll reach
: enlightment. So I won't
: explain that to you ;-)

I am the tree that falls in the forrest, if I'm deaf, do I still make
a sound?

: I meant that Jan's forthcoming series that heavily touch kvm memory
: registration (the beautiful way)
: is unlikely to get into stable. So I'd advocate applying this patch
: (the said ugly way) to stable.
: Otherwise we'll get -vga std broken for good.

OK.  That makes sense...

Warner

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:47                   ` Paul Brook
@ 2009-04-08 18:07                     ` Glauber Costa
  0 siblings, 0 replies; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 18:07 UTC (permalink / raw)
  To: Paul Brook; +Cc: glommer, qemu-devel

On Wed, Apr 8, 2009 at 2:47 PM, Paul Brook <paul@codesourcery.com> wrote:
> On Wednesday 08 April 2009, M. Warner Losh wrote:
>> In message: <5d6222a80904081025j1644b6bay17d1ff942247971@mail.gmail.com>
>>
>>             Glauber Costa <glommer@gmail.com> writes:
>> : In all seriousness, no matter how good your series for improving kvm
>> : registration
>> : are, it is unlikely to hit the stable branch. So I believe a good deal
>> : would have something
>> : on the line of this patch applied to stable, leaving unstable as is,
>> : until we can find a better
>> : solution.
>>
>> Huh?  It is unlikely to be merged into stable, so let's commit it to
>> stable?
>
> I think the suggestion is that we apply an ugly hack on just the stable
> branch, and fix it properly later on the development.
>
> FWIW I think this is a bad idea. We should never fix something on the stable
> branch that isn't also fixed on the development branch. If the changes are
> causing problems, then my suggestion is to revert the original change (i.e.
> the support for >64k roms).

Just checked, and the > 64 roms patch is not on stable branch.

So this is a non issue.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:20             ` Jan Kiszka
  2009-04-08 17:23               ` Glauber Costa
  2009-04-08 17:25               ` Glauber Costa
@ 2009-04-08 18:57               ` Anthony Liguori
  2009-04-08 19:03                 ` Glauber Costa
  2 siblings, 1 reply; 20+ messages in thread
From: Anthony Liguori @ 2009-04-08 18:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jan Kiszka, Glauber Costa

Jan Kiszka wrote:
> Easter is coming - if I find my eggs quickly, there might be some time
> left... ;)
>
> I leave the decision up to the maintainers if we should merge this
> workaround first, or wait a bit longer until we know for sure if we can
> fix it in a cleaner way.
>   

I'd rather wait for a proper fix so I'll let Glauber's patch age a bit 
in my queue.

Regards,

Anthony Liguori
>   

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 17:25               ` Glauber Costa
  2009-04-08 17:39                 ` M. Warner Losh
@ 2009-04-08 18:58                 ` Anthony Liguori
  1 sibling, 0 replies; 20+ messages in thread
From: Anthony Liguori @ 2009-04-08 18:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: Glauber Costa

Glauber Costa wrote:
>
> In all seriousness, no matter how good your series for improving kvm
> registration
> are, it is unlikely to hit the stable branch. So I believe a good deal
> would have something
> on the line of this patch applied to stable, leaving unstable as is,
> until we can find a better
> solution.
>
> What do you think, anthony?
>   

As mentioned on IRC, we don't have this problem in stable.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] Re: start qemu failed with --enable-kvm -vga std
  2009-04-08 18:57               ` Anthony Liguori
@ 2009-04-08 19:03                 ` Glauber Costa
  0 siblings, 0 replies; 20+ messages in thread
From: Glauber Costa @ 2009-04-08 19:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jan Kiszka, Glauber Costa

On Wed, Apr 8, 2009 at 3:57 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> Jan Kiszka wrote:
>>
>> Easter is coming - if I find my eggs quickly, there might be some time
>> left... ;)
>>
>> I leave the decision up to the maintainers if we should merge this
>> workaround first, or wait a bit longer until we know for sure if we can
>> fix it in a cleaner way.
>>
>
> I'd rather wait for a proper fix so I'll let Glauber's patch age a bit in my
> queue.

Beware.

If it ages too much, I'll want to drink it.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2009-04-08 19:03 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-08  3:11 [Qemu-devel] start qemu failed with --enable-kvm -vga std Peng Huang
2009-04-08 12:27 ` Paul Brook
2009-04-08 13:12 ` Anthony Liguori
2009-04-08 13:25   ` Glauber Costa
2009-04-08 16:34     ` [Qemu-devel] " Jan Kiszka
2009-04-08 16:41       ` Glauber Costa
2009-04-08 17:02         ` Jan Kiszka
2009-04-08 17:06           ` Glauber Costa
2009-04-08 17:20             ` Jan Kiszka
2009-04-08 17:23               ` Glauber Costa
2009-04-08 17:25               ` Glauber Costa
2009-04-08 17:39                 ` M. Warner Losh
2009-04-08 17:44                   ` Glauber Costa
2009-04-08 17:57                     ` M. Warner Losh
2009-04-08 17:47                   ` Paul Brook
2009-04-08 18:07                     ` Glauber Costa
2009-04-08 18:58                 ` Anthony Liguori
2009-04-08 18:57               ` Anthony Liguori
2009-04-08 19:03                 ` Glauber Costa
2009-04-08 17:31       ` Paul Brook

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).