lkml.org 
[lkml]   [2020]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] irqchip/gic-v3-its: Disable vSGI upon (CPUIF < v4.1) detection
On 2020-11-12 14:40, Lorenzo Pieralisi wrote:
> On Thu, Nov 12, 2020 at 09:36:10AM +0000, Marc Zyngier wrote:
>> Hi Lorenzo,
>>
>> On 2020-11-11 16:28, Lorenzo Pieralisi wrote:
>> > GIC CPU interfaces versions predating GIC v4.1 were not built to
>> > accommodate vINTID within the vSGI range; as reported in the GIC
>> > specifications (8.2 "Changes to the CPU interface"), it is
>> > CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with
>> > ID_AA64PFR0_EL1.GIC == b0001.
>>
>> Hmmm. This goes against the very reason v4.1 was designed the way
>> it is, which was that all existing implementation supporting GICv4.0
>> would seamlessly let virtual SGIs in, and it would "just work".
>>
>> If we start enforcing this, I question the very design of the
>> architecture,
>> because we could have done so much better by changing the CPU
>> interface.
>>
>> What has changed in two years? Have you spotted a fundamental problem?
>
> Hi Marc,
>
> long story short: there are systems being designed with this
> configuration, vSGIs may or may not work on them, to prevent
> *potential* misbehaviour I am disabling vSGIs, I am not fixing
> anything, it is belt and braces.
>
>> My concern is that if we prevent it, we're going to end-up with quirks
>> allowing it anyway, because people will realise that it actually
>> works.
>
> We may wait and fix it *if* this breaks, I would argue though that at
> that point it is not a quirk since architecturally we know that vSGIs
> may not work in this configuration.

I don't mind either way, as I doubt I'll see this kind of system any
time
soon. I'm just mildly annoyed at the missed opportunity to do something
better...

>
>> In the meantime, to the meat of the change:
>>
>> >
>> > Check the GIC CPUIF version through the arm64 capabilities
>> > infrastructure and disable vSGIs if a CPUIF version < 4.1 is
>> > detected to prevent using vSGIs on systems where they may
>> > misbehave.
>> >
>> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>> > Cc: Marc Zyngier <maz@kernel.org>
>> > ---
>> > drivers/irqchip/irq-gic-v3-its.c | 20 +++++++++++++++++++-
>> > 1 file changed, 19 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> > b/drivers/irqchip/irq-gic-v3-its.c
>> > index 0fec31931e11..6ed4ba60ba7e 100644
>> > --- a/drivers/irqchip/irq-gic-v3-its.c
>> > +++ b/drivers/irqchip/irq-gic-v3-its.c
>> > @@ -39,6 +39,20 @@
>> >
>> > #include "irq-gic-common.h"
>> >
>> > +#ifdef CONFIG_ARM64
>> > +#include <asm/cpufeature.h>
>> > +
>> > +static inline bool gic_cpuif_has_vsgi(void)
>> > +{
>> > + return cpus_have_const_cap(ARM64_HAS_GIC_CPUIF_VSGI);
>> > +}
>> > +#else
>> > +static inline bool gic_cpuif_has_vsgi(void)
>> > +{
>> > + return false;
>> > +}
>> > +#endif
>> > +
>> > #define ITS_FLAGS_CMDQ_NEEDS_FLUSHING (1ULL << 0)
>> > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
>> > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
>> > @@ -5415,7 +5429,11 @@ int __init its_init(struct fwnode_handle
>> > *handle, struct rdists *rdists,
>> > if (has_v4 & rdists->has_vlpis) {
>> > const struct irq_domain_ops *sgi_ops;
>> >
>> > - if (has_v4_1)
>> > + /*
>> > + * Enable vSGIs only if the ITS and the
>> > + * GIC CPUIF support them.
>> > + */
>> > + if (has_v4_1 && gic_cpuif_has_vsgi())
>> > sgi_ops = &its_sgi_domain_ops;
>> > else
>> > sgi_ops = NULL;
>>
>> Is that enough?
>
> No, I obviously missed the VGIC bits built on top of
> GICD_TYPER2.nASSGIcap.
>
>> KVM is still going to expose GICD_TYPER2.nASSGIcap, making things even
>> more confusing for the guest: it will be able to select active-less
>> SGIs
>> via GICD_CTLR.nASSGIreq, and if I'm not mistaken, we'd still try to
>> switch
>> to HW-backed SGIs, leading to some *very* unpleasant things in
>> gic_v4_enable_vsgis().
>
> Yes (AFAICS GICD_TYPER2.nASSGIcap is not in the public specs though,
> that's why I missed it while vetting architectural state that is
> affecting vSGIs).

You can find it in the errata to the spec (I just checked the October
2020
version). I doubt it is public though, and people have been asking for
this
update to be published for a while now.

> I should change the logic in vgic_mmio_{uaccess}_write_v3_misc() to
> handle it properly - to redefine the logic around
>
> kvm_vgic_global_state.has_gicv4_1
>
> somehow.

You probably need a separate predicate, indicating HW-baked vSGI
support.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2020-11-12 16:40    [W:0.263 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site