[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/8] x86/hvm: Only populate info->cr2 for #PF in hvm_get_pending_event()


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Jinoh Kang <jinoh.kang.kr@xxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Wed, 30 Aug 2023 14:48:56 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=yZkvpPDYAJc09CEQjtGnfrmOK3SfiysM9KzUYNFS36U=; b=NWUGw5HhHpHumppGZEYNL/FHHnXWu9u7mAWp7PJgyEXPdXJG42+pbLGRvZb3NOrb4GCzyl1JDPfekpqwQI6UhJysCqJPcCRODjmvGyDk07azimfLuXDmX8w/t7aV1Ju2QOZv+gsh72Nzge5tSzC4lEtIBAoTjS2Oyds8YjV/X4OSWefVYsdYYZNJVL+g+VszFsSapm2HoqaDFmfifbf8fsL1oquaNnlcovxGWBklUbZmcHAUfdzQ7OReUgDuk91R/RJnAa0pd0YNQNs1JOtROBxDvmOo/00BaZ5daXAeRpbgm2DRyl1GhypcPqintiWNnsmyXKs0tUftUmGpvWscag==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FVEHgUR/2lLpyYJYVkGbh+a+b6nOgysGKCmUKASzT7SdKOBA6QxYnODIPg/YOVHTnaPPemwoA7B9zqkMQ9v2pa+Vp+o3EHIG+IYHHdBUe0+Jsx2gz/bGu4zvoHBrhvykTFAN6NHv0G14tytMzVTc7LqZrTBZ+HbLh7dnVv9ozgP2hylmXrp7bcLIhwAArmUNjxHmdv2JB0wggmtqIE8vtugLJ1xA/8g/OnCjVLYGkIkd/i1hBj9wYVJ+WePpcEs5TgYqPc8yPkaP9zjjYmOdvCAKKjez5Ec/clmCh4zmiVWIA6AuFJLEEUhVCOhdqOEG9op4OpDUI8FjVRiDeR78Gg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 30 Aug 2023 13:49:16 +0000
  • Ironport-data: A9a23:iOgMfKJOlYRyizLgFE+RX5QlxSXFcZb7ZxGr2PjKsXjdYENShWcFz jYbUWuGOKuPN2X9KYh0a46+90wD6pCAyoA1T1BlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHvykU7Ss1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpKrfrZwP9TlK6q4mhA7gdmPaojUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c5XJG537 cM0IgwBd1exg7uZ3qyhDapF05FLwMnDZOvzu1lG5BSAV7MKZM6GRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dmpTGMl2Sd05C0WDbRUvWMSd9YgQCzo WXe8n6iKhobKMae2XyO9XfEaurnxHmjA9NJRebinhJsqFaD6EYBCwVMaQSci6ewun63ae4PE lNBr0LCqoB3riRHVOLVXRe1vXqFtR40QMdLHqsx7wTl4qjV5QGZQGsNSDEHa8YOu8o/RDhs3 ViM9/v2ARR/vbvTTmiSnp+IpDa7IgAJLmsPYyAVQA9D6N7myKk6jwnGT9JqOKS0ktH4Fzz2z z2Q6iM5gt07ldYKza6y+VnNnhqmp4TFQwA44AnaRCSu6QYRTJ6oYcmk5EbW6d5ELZ2FVR+Rs X4cgc+c4esSS5aXm0SwrP4lGbio47OJNWPaiFs2RZ05rW3yoDikYJxa5yx4KAFxKMEYdDT1Y UjV/wRM+JtUO3jsZqhyC26sN/kXIWHbPYyNfpjpghBmO/CdqCfvEPlSWHOt
  • Ironport-hdrordr: A9a23:tTqjuqO8ps/uxcBcTsCjsMiBIKoaSvp037BL7SFMoHluGPBw+P rBoB12726RtN9pYhwdcIm7Scq9qBDnlaKdg7N8AV7KZmCPhILPFvAE0WKI+UyDJ8SRzIFgPJ BbAs1D4Y3LZmRHsQ==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 30/08/2023 2:41 pm, Jan Beulich wrote:
> On 24.08.2023 17:25, Jinoh Kang wrote:
>> Prepare for an upcoming patch that overloads the 'cr2' field for #DB.
> Seeing the subsequent change and the fact that earlier on Andrew didn't
> need such an adjustment, I'm afraid I can't see the need for this change,
> and the one sentence above also doesn't answer the "Why?", but only the
> "What?"

I agree WRT the commit message.

I don't see why, but I also didn't spot this specific bug so I can't
rule out a bug in my original series.

That said, my original series was RFC because of the Monitor breakage
and didn't get much testing.

>
> Jan
>
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -503,9 +503,14 @@ void hvm_migrate_pirqs(struct vcpu *v)
>>  
>>  static bool hvm_get_pending_event(struct vcpu *v, struct x86_event *info)
>>  {
>> -    info->cr2 = v->arch.hvm.guest_cr[2];
>> +    if ( !alternative_call(hvm_funcs.get_pending_event, v, info) )
>> +        return false;
>> +
>> +    if ( info->type == X86_EVENTTYPE_HW_EXCEPTION &&
>> +         info->vector == X86_EXC_PF )
>> +        info->cr2 = v->arch.hvm.guest_cr[2];

For the change itself, this needs pushing down into the vmx/svm hooks,
because guest_cr[2] has different liveness between Intel and AMD, and
this callpath needs to work for both scheduled-in and scheduled-out guests.

On AMD, I think you need to pull it straight out of the VMCB, rather
than relying on this being correct for a scheduled-in guest.

~Andrew



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.