LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Suthikulpanit, Suravee" <Suravee.Suthikulpanit@amd.com>
To: "Jan H. Schönherr" <jschoenh@amazon.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc: "joro@8bytes.org" <joro@8bytes.org>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"rkrcmar@redhat.com" <rkrcmar@redhat.com>
Subject: Re: [PATCH 4/6] kvm: lapic: Add apicv activate/deactivate helper function
Date: Mon, 15 Jul 2019 22:35:00 +0000	[thread overview]
Message-ID: <499a4947-bb65-a0ea-cf16-088a3a4b1ebd@amd.com> (raw)
In-Reply-To: <813c8c15-af74-6d77-4a30-2e0ee4b05006@amazon.de>

Jan,

On 5/8/2019 5:27 PM, Jan H. Schönherr wrote:
> On 22/03/2019 12.57, Suthikulpanit, Suravee wrote:
>> Introduce a helper function for setting lapic parameters when
>> activate/deactivate apicv.
>>
>> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com>
>> ---
>>   arch/x86/kvm/lapic.c | 23 ++++++++++++++++++-----
>>   arch/x86/kvm/lapic.h |  1 +
>>   2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 95295cf81283..9c5cd1a98928 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -2126,6 +2126,22 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
>>
>>   }
>>
>> +void kvm_apic_update_apicv(struct kvm_vcpu *vcpu)
>> +{
>> +     struct kvm_lapic *apic = vcpu->arch.apic;
>> +     bool active = vcpu->arch.apicv_active;
>> +
>> +     if (active) {
>> +             /* irr_pending is always true when apicv is activated. */
>> +             apic->irr_pending = true;
>> +             apic->isr_count = 1;
>> +     } else {
>> +             apic->irr_pending = !!count_vectors(apic->regs + APIC_IRR);
> What about:
>                  apic->irr_pending = apic_search_irr(apic) != -1;
> instead? (more in line with the logic in apic_clear_irr())

Sure, that works also.

> Related to this, I wonder if we need to ensure to execute
> kvm_x86_ops->sync_pir_to_irr() just before apicv_active transitions
> from true to false, so that we don't miss an interrupt and irr_pending
> is set correctly in this function (on Intel at least).

I'm not sure about the PIR on Intel, but AMD since AMD IOMMU hardware
directly updates the vAPIC backing page, the driver should not need
to worry.

> Hmm... there seems to be other stuff as well, that depends on
> vcpu->arch.apicv_active, which is not updated on a transition. For
> example: posted interrupts, 

You are right. We need to also handle the posted-interrupt
activate/deactivate. I am also doing this in V2.

> CR8 intercept, 

When APICv is deactivated, the update_cr8_intercept() should handle
updating of CR8 interception.

> and a potential asymmetry via avic_vcpu_load()/avic_vcpu_put()
> because the bottom half of just one of the two functions may be
> skipped
Not sure if I understand the concern that you mentioned here.
However, once we add the IOMMU activation/deactivation code for
posted-interrupt, we should not need to worry about calling
avic_update_iommu_vcpu_affinity() at the end of the functions
here.

Thanks,
Suravee
> Regards
> Jan
> 

  reply	other threads:[~2019-07-15 22:35 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-22 11:57 [PATCH 0/6] KVM/x86: Add workaround to support ExtINT with AVIC Suthikulpanit, Suravee
2019-03-22 11:57 ` [PATCH 1/6] KVM: x86: Add callback functions for handling APIC ID, DFR and LDR update Suthikulpanit, Suravee
2019-07-03 21:16   ` Paolo Bonzini
2019-07-17 19:44     ` Suthikulpanit, Suravee
2019-03-22 11:57 ` [PATCH 2/6] svm: Add AMD AVIC handlers for " Suthikulpanit, Suravee
2019-03-22 11:57 ` [PATCH 3/6] svm: Add support for APIC_ACCESS_PAGE_PRIVATE_MEMSLOT setup/destroy Suthikulpanit, Suravee
2019-05-08 19:14   ` Jan H. Schönherr
2019-06-30 16:19     ` Suthikulpanit, Suravee
2019-03-22 11:57 ` [PATCH 4/6] kvm: lapic: Add apicv activate/deactivate helper function Suthikulpanit, Suravee
2019-05-08 22:27   ` Jan H. Schönherr
2019-07-15 22:35     ` Suthikulpanit, Suravee [this message]
2019-03-22 11:57 ` [PATCH 5/6] KVM: x86: Add interface for run-time activate/de-activate APIC virtualization Suthikulpanit, Suravee
2019-05-08 20:45   ` Jan H. Schönherr
2019-03-22 11:57 ` [PATCH 6/6] svm: Temporary deactivate AVIC during ExtINT handling Suthikulpanit, Suravee
2019-05-08 17:37   ` Jan H. Schönherr
2019-06-03 18:58     ` Suthikulpanit, Suravee
2019-04-04 21:30 ` [PATCH 0/6] KVM/x86: Add workaround to support ExtINT with AVIC rkrcmar
2019-04-04 22:06 ` rkrcmar

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=499a4947-bb65-a0ea-cf16-088a3a4b1ebd@amd.com \
    --to=suravee.suthikulpanit@amd.com \
    --cc=joro@8bytes.org \
    --cc=jschoenh@amazon.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    --subject='Re: [PATCH 4/6] kvm: lapic: Add apicv activate/deactivate helper function' \
    /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

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