All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@web.de>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] kvm: fix Win2k boot without KVM
Date: Sat, 03 Nov 2012 20:37:51 +0100	[thread overview]
Message-ID: <5095728F.2010900@web.de> (raw)
In-Reply-To: <CAAu8pHsZd00xt8CfWjgAPDhFYaoabvPe59TTa4xD2WLS+a2dLQ@mail.gmail.com>

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

On 2012-11-03 20:26, Blue Swirl wrote:
> On Sat, Nov 3, 2012 at 7:10 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> On 2012-11-03 20:03, Jan Kiszka wrote:
>>> On 2012-11-03 19:56, Blue Swirl wrote:
>>>> On Sat, Nov 3, 2012 at 6:51 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>> On 2012-11-03 19:49, Blue Swirl wrote:
>>>>>> Ignore accesses to VAPIC when kvmvapic is not enabled.
>>>>>>
>>>>>> Cc: Jan Kiszka <jan.kiszka@web.de>
>>>>>> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
>>>>>> ---
>>>>>>  hw/kvmvapic.c |    7 ++++---
>>>>>>  1 files changed, 4 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/kvmvapic.c b/hw/kvmvapic.c
>>>>>> index dc111ee..a97d532 100644
>>>>>> --- a/hw/kvmvapic.c
>>>>>> +++ b/hw/kvmvapic.c
>>>>>> @@ -612,6 +612,9 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
>>>>>>      hwaddr rom_paddr;
>>>>>>      VAPICROMState *s = opaque;
>>>>>>
>>>>>> +    if (!kvm_irqchip_in_kernel()) {
>>>>>> +        return;
>>>>>> +    }
>>>>>>      cpu_synchronize_state(env);
>>>>>>
>>>>>>      /*
>>>>>> @@ -665,9 +668,7 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
>>>>>>          break;
>>>>>>      default:
>>>>>>      case 4:
>>>>>> -        if (!kvm_irqchip_in_kernel()) {
>>>>>> -            apic_poll_irq(env->apic_state);
>>>>>> -        }
>>>>>> +        apic_poll_irq(env->apic_state);
>>>>>>          break;
>>>>>>      }
>>>>>>  }
>>>>>>
>>>>>
>>>>> NACK, I'm already debugging the true reason (related to code patching).
>>>>
>>>> This is a minimal fix that lets Win2k boot, now it does not work at
>>>> all. I think it should be applied for 1.3, it can be reverted when
>>>> (if) you find a better fix. There's no hurry though.
>>>
>>> If you want to disable it, flip apic.vapic for !kvm_enabled. Your patch
>>> affects user space APIC with KVM as well, though that is perfectly fine.
>>>
>>> But first of all give this some days as I just started.
>>
>> ...even more as this regression may not be related to the introduction
>> of the kvmvapic: My original test case for the kvmvapic under TCG,
>> WinXP, is now also broken, causing a segfault too.
>>
>> What I'm seeing is that tb_invalidate_phys_page_range in
>> patch_instruction no longer seems to detect that the currently executed
>> tb was just changed. Any ideas what may cause this are welcome.
> 
> My theory is that the kvmvapic ROM tries to make the PIO hypercalls,
> but the PIO devices in QEMU are not ready, maybe not initialized at
> all.

You are on the wrong track: All is set up, the first TPR accesses are
happening, and the kvmvapic is trying to patch them away. In TCG mode,
this requires a flush of the current TB afterward. And this somehow
fails, causing various issues as executing resumes at invalid addresses.

Jan

PS: A good hash is e.g. b34bd5e5c8f356ec206e5a306ee3a9b6f42c4315, long
after the kvmvapic merge.

PPS: Bisecting in QEMU is no fun - too many transient build breakages.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

  reply	other threads:[~2012-11-03 19:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-03 18:49 [Qemu-devel] [PATCH] kvm: fix Win2k boot without KVM Blue Swirl
2012-11-03 18:51 ` Jan Kiszka
2012-11-03 18:56   ` Blue Swirl
2012-11-03 19:03     ` Jan Kiszka
2012-11-03 19:10       ` Blue Swirl
2012-11-03 19:10       ` Jan Kiszka
2012-11-03 19:26         ` Blue Swirl
2012-11-03 19:37           ` Jan Kiszka [this message]
2012-11-03 20:57             ` Jan Kiszka
2012-11-03 21:00               ` Jan Kiszka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5095728F.2010900@web.de \
    --to=jan.kiszka@web.de \
    --cc=blauwirbel@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.