[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 7/7] xen/arm: Sanitize CTR_EL0 and emulate it if needed



Hi Bertrand,

On 06/09/2021 09:29, Bertrand Marquis wrote:
On 3 Sep 2021, at 23:49, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:

On Tue, 31 Aug 2021, Bertrand Marquis wrote:
Hi Julien,

On 31 Aug 2021, at 15:47, Julien Grall <julien@xxxxxxx> wrote:



On 31/08/2021 14:17, Bertrand Marquis wrote:
Hi Julien,

Hi Bertrand,

On 27 Aug 2021, at 16:05, Julien Grall <julien@xxxxxxx> wrote:

Hi Bertrand,

On 25/08/2021 14:18, Bertrand Marquis wrote:
Sanitize CTR_EL0 value between cores.
In most cases different values will taint Xen but if different
i-cache policies are found, we choose the one which will be compatible
between all cores in terms of invalidation/data cache flushing strategy.

I understand that all the CPUs in Xen needs to agree on the cache flush 
strategy. However...

In this case we need to activate the TID2 bit in HCR to emulate the
TCR_EL0 register for guests. This patch is not activating TID2 bit all
the time to limit the overhead when possible.

as we discussed in an earlier version, a vCPU is unlikely (at least in 
short/medium) to be able move across pCPU of different type. So the vCPU would 
be pinned to a set of pCPUs. IOW, the guest would have to be big.LITTLE aware 
and therefore would be able to do its own strategy decision.

So I think we should be able to get away from trappings the registers.
I do agree that we should be able to get away from that in the long term once
we have cpupools properly set but right now this is the only way to have
something useable (I will not say right).
I will work on finding a way to setup properly cpupools (or something else as
we discussed earlier) but in the short term I think this is the best we can do.

My concern is you are making look like Xen will be able to deal nicely with 
big.LITTLE when in fact there are a lot more potential issue by allow a vCPU 
moving accross pCPU of different type (the errata is one example).

I agree and this is why Xen is tainted.


An other solution would be to discard this patch from the serie for now until
I have worked a proper solution for this case.
Should we discard or merge or do you have an other idea ?
Please correct me if I am wrong, at the moment, it doesn't look like this patch 
will be part of the longer plan. If so, then I think it should be parked for 
now.

Not sure it depends on what the final solution would be but this is highly 
possible I agree.


This would also have the advantage to avoid spending too much time on resolving 
the emulation issue I mentioned in my previous answer.

No need to resend a new version of this series yet. You can wait until the rest 
of the series get more feedback.

Ok, I will wait for feedback and next serie will not include this patch anymore.

Would it be worth keeping just the part that sanitizes ctr, without any
of the emulation stuff? That way we could still detect cases where there
is a mismatch between CPUs, print a useful message and taint Xen.

That’s a good idea, it means removing the emulation part and just keep the 
sanitization.

@Julien: would you be ok with that ?

I actually thought about suggesting it before Stefano did it. So I am OK with that.


Should I send a v4 or should we use Stefano’s patch directly instead ?

I would suggest to send a v4. This needs a signed-off-by from Stefano and a new commit message.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.