Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
Excerpts from Paul Mackerras's message of June 30, 2020 6:26 pm:
> On Tue, Jun 30, 2020 at 03:35:08PM +1000, Nicholas Piggin wrote:
>> Excerpts from Paul Mackerras's message of June 30, 2020 12:27 pm:
>> > On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote:
>> >> KVM guests have certain restrictions and performance quirks when
>> >> using doorbells. This patch tests for KVM environment in doorbell
>> >> setup, and optimises IPI performance:
>> >>
>> >> - PowerVM guests may now use doorbells even if they are secure.
>> >>
>> >> - KVM guests no longer use doorbells if XIVE is available.
>> >
>> > It seems, from the fact that you completely remove
>> > kvm_para_available(), that you perhaps haven't tried building with
>> > CONFIG_KVM_GUEST=y.
>>
>> It's still there and builds:
>
> OK, good, I missed that.
>
>> static inline int kvm_para_available(void)
>> {
>> return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
>> }
>>
>> but...
>>
>> > Somewhat confusingly, that option is not used or
>> > needed when building for a PAPR guest (i.e. the "pseries" platform)
>> > but is used on non-IBM platforms using the "epapr" hypervisor
>> > interface.
>>
>> ... is_kvm_guest() returns false on !PSERIES now.
>
> And therefore kvm_para_available() returns false on all the platforms
> where the code that depends on it could actually be used.
>
> It's not correct to assume that !PSERIES means not a KVM guest.
Yep, thanks for catching it.
>> Not intended
>> to break EPAPR. I'm not sure of a good way to share this between
>> EPAPR and PSERIES, I might just make a copy of it but I'll see.
>
> OK, so you're doing a new version?
Just sent.
Thanks,
Nick
Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
On Tue, Jun 30, 2020 at 03:35:08PM +1000, Nicholas Piggin wrote:
> Excerpts from Paul Mackerras's message of June 30, 2020 12:27 pm:
> > On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote:
> >> KVM guests have certain restrictions and performance quirks when
> >> using doorbells. This patch tests for KVM environment in doorbell
> >> setup, and optimises IPI performance:
> >>
> >> - PowerVM guests may now use doorbells even if they are secure.
> >>
> >> - KVM guests no longer use doorbells if XIVE is available.
> >
> > It seems, from the fact that you completely remove
> > kvm_para_available(), that you perhaps haven't tried building with
> > CONFIG_KVM_GUEST=y.
>
> It's still there and builds:
OK, good, I missed that.
> static inline int kvm_para_available(void)
> {
> return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
> }
>
> but...
>
> > Somewhat confusingly, that option is not used or
> > needed when building for a PAPR guest (i.e. the "pseries" platform)
> > but is used on non-IBM platforms using the "epapr" hypervisor
> > interface.
>
> ... is_kvm_guest() returns false on !PSERIES now.
And therefore kvm_para_available() returns false on all the platforms
where the code that depends on it could actually be used.
It's not correct to assume that !PSERIES means not a KVM guest.
> Not intended
> to break EPAPR. I'm not sure of a good way to share this between
> EPAPR and PSERIES, I might just make a copy of it but I'll see.
OK, so you're doing a new version?
Regards,
Paul.
Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
Excerpts from Paul Mackerras's message of June 30, 2020 12:27 pm:
> On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote:
>> KVM guests have certain restrictions and performance quirks when
>> using doorbells. This patch tests for KVM environment in doorbell
>> setup, and optimises IPI performance:
>>
>> - PowerVM guests may now use doorbells even if they are secure.
>>
>> - KVM guests no longer use doorbells if XIVE is available.
>
> It seems, from the fact that you completely remove
> kvm_para_available(), that you perhaps haven't tried building with
> CONFIG_KVM_GUEST=y.
It's still there and builds:
static inline int kvm_para_available(void)
{
return IS_ENABLED(CONFIG_KVM_GUEST) && is_kvm_guest();
}
but...
> Somewhat confusingly, that option is not used or
> needed when building for a PAPR guest (i.e. the "pseries" platform)
> but is used on non-IBM platforms using the "epapr" hypervisor
> interface.
... is_kvm_guest() returns false on !PSERIES now. Not intended
to break EPAPR. I'm not sure of a good way to share this between
EPAPR and PSERIES, I might just make a copy of it but I'll see.
Thanks,
Nick
Re: [PATCH 3/3] powerpc/pseries: Add KVM guest doorbell restrictions
On Sun, Jun 28, 2020 at 01:04:28AM +1000, Nicholas Piggin wrote: > KVM guests have certain restrictions and performance quirks when > using doorbells. This patch tests for KVM environment in doorbell > setup, and optimises IPI performance: > > - PowerVM guests may now use doorbells even if they are secure. > > - KVM guests no longer use doorbells if XIVE is available. It seems, from the fact that you completely remove kvm_para_available(), that you perhaps haven't tried building with CONFIG_KVM_GUEST=y. Somewhat confusingly, that option is not used or needed when building for a PAPR guest (i.e. the "pseries" platform) but is used on non-IBM platforms using the "epapr" hypervisor interface. If you did intend to remove support for the epapr hypervisor interface then that should have been talked about in the commit message (and would I expect be controversial). So NAK on the kvm_para_available() removal. Paul.
