Hi,
On 4/28/21 1:06 PM, Alex Bennée wrote:
> Marc Zyngier <maz@xxxxxxxxxx> writes:
>
>> On 2021-04-28 11:18, Alex Bennée wrote:
>>> A few of the its-trigger tests rely on IMPDEF behaviour where caches
>>> aren't flushed before invall events. However TCG emulation doesn't
>>> model any invall behaviour and as we can't probe for it we need to be
>>> told. Split the test into a KVM and TCG variant and skip the invall
>>> tests when under TCG.
>>> Signed-off-by: Alex Bennée <alex.bennee@xxxxxxxxxx>
>>> Cc: Shashi Mallela <shashi.mallela@xxxxxxxxxx>
>>> ---
>>> arm/gic.c | 60 +++++++++++++++++++++++++++--------------------
>>> arm/unittests.cfg | 11 ++++++++-
>>> 2 files changed, 45 insertions(+), 26 deletions(-)
>>> diff --git a/arm/gic.c b/arm/gic.c
>>> index 98135ef..96a329d 100644
>>> --- a/arm/gic.c
>>> +++ b/arm/gic.c
>>> @@ -36,6 +36,7 @@ static struct gic *gic;
>>> static int acked[NR_CPUS], spurious[NR_CPUS];
>>> static int irq_sender[NR_CPUS], irq_number[NR_CPUS];
>>> static cpumask_t ready;
>>> +static bool under_tcg;
>>> static void nr_cpu_check(int nr)
>>> {
>>> @@ -734,32 +735,38 @@ static void test_its_trigger(void)
>>> /*
>>> * re-enable the LPI but willingly do not call invall
>>> * so the change in config is not taken into account.
>>> - * The LPI should not hit
>>> + * The LPI should not hit. This does however depend on
>>> + * implementation defined behaviour - under QEMU TCG emulation
>>> + * it can quite correctly process the event directly.
>> It looks to me that you are using an IMPDEF behaviour of *TCG*
>> here. The programming model mandates that there is an invalidation
>> if you change the configuration of the LPI.
> But does it mandate that the LPI cannot be sent until the invalidation?
I think Marc is referring to this section of the GIC architecture (Arm IHI 0069F,
page 5-82, I've highlighted the interesting bits):
"A Redistributor can cache the information from the LPI Configuration tables
pointed to by GICR_PROPBASER, when GICR_CTLR.EnableLPI == 1, subject to all of the
following rules:
* Whether or not one or more caches are present is IMPLEMENTATION DEFINED. Where
at least one cache is present, the structure and size is IMPLEMENTATION DEFINED.
* An LPI Configuration table entry might be allocated into the cache at any time.
* A cached LPI Configuration table entry is not guaranteed to remain in the cache.
* A cached LPI Configuration table entry *is not guaranteed to remain incoherent
with memory*.
* A change to the LPI configuration *is not guaranteed to be visible until an
appropriate invalidation operation has completed*"
I interpret that as that an INVALL guarantees that a change is visible, but it the
change can become visible even without the INVALL.
The test relies on the fact that changes to the LPI tables are not visible *under
KVM* until the INVALL command, but that's not necessarily the case on real
hardware. To match the spec, I think the test "dev2/eventid=20 still does not
trigger any LPI" should be removed and the stats reset should take place before
the configuration for LPI 8195 is set to the default.
Thanks,
Alex
