On 2/4/19 1:50 AM, David Gibson wrote:
> On Wed, Jan 23, 2019 at 05:24:13PM +0100, Cédric Le Goater wrote:
>> On 1/22/19 5:56 AM, Paul Mackerras wrote:
>>> On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
>>>> We will have different KVM devices for interrupts, one for the
>>>> XICS-over-XIVE mode and one for the XIVE native exploitation
>>>> mode. Let's add some checks to make sure we are not mixing the
>>>> interfaces in KVM.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@xxxxxxxx>
>>>> ---
>>>> arch/powerpc/kvm/book3s_xive.c | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>>>> index f78d002f0fe0..8a4fa45f07f8 100644
>>>> --- a/arch/powerpc/kvm/book3s_xive.c
>>>> +++ b/arch/powerpc/kvm/book3s_xive.c
>>>> @@ -819,6 +819,9 @@ u64 kvmppc_xive_get_icp(struct kvm_vcpu *vcpu)
>>>> {
>>>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>>>>
>>>> + if (!kvmppc_xics_enabled(vcpu))
>>>> + return -EPERM;
>>>> +
>>>> if (!xc)
>>>> return 0;
>>>>
>>>> @@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)
>>>> u8 cppr, mfrr;
>>>> u32 xisr;
>>>>
>>>> + if (!kvmppc_xics_enabled(vcpu))
>>>> + return -EPERM;
>>>> +
>>>> if (!xc || !xive)
>>>> return -ENOENT;
>>>
>>> I can't see how these new checks could ever trigger in the code as it
>>> stands. Is there a way at present?
>>
>> It would require some custom QEMU doing silly things : create the XICS
>> KVM device, and then call kvm_get_one_reg(KVM_REG_PPC_ICP_STATE) or
>> kvm_set_one_reg(icp->cs, KVM_REG_PPC_ICP_STATE) without connecting the
>> vCPU to its presenter.
>>
>> Today, you get a ENOENT.
>
> TBH, ENOENT seems fine to me.
>
>>> Do following patches ever add a path where the new checks could trigger,
>>> or is this just an excess of caution?
>>
>> With the following patches, QEMU could to do something even more silly,
>> which is to mix the interrupt mode interfaces : create a KVM XICS device
>> and call KVM CPU ioctls of the KVM XIVE device, or the opposite.
>
> AFAICT, like above, that won't really differ from calling the XIVE CPU
> ioctl()s when no irqchip is set up at all, and should be covered by
> just a !xive check.
we can drop that patch. It does not bring much.
Thanks,
C.
>
>>
>>> (Your patch description should ideally have answered these questions > for me.)
>>
>> Yes. I also think that I introduced this patch to early in the series.
>> It make more sense when the XICS and the XIVE KVM devices are available.
>>
>> Thanks,
>>
>> C.
>>
>
