[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

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).

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.

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.


If we get interrupted, someone could program CSSELR differently and the next 
will not be reflecting what the guest actually wants to do

AFAICT, CSSELR is preserved during the context switch of vCPU. So that someone would have to be Xen, right?

If so, what you describe would also be an issue even if we didn't trap the register. Therefore, if Xen would ever use CSSELR, then that code would need to save the value, use the register and then restore the value with preemption disabled.

The code is not preemptible right now so this cannot be an issue but I added the
  comment more as a warning.

This is not something from the documentation, this is because value written
in CSSELR is defining what is read from CCSIDR

+            WRITE_SYSREG(v->arch.csselr, CSSELR_EL1);
+            set_user_reg(regs, regidx, READ_SYSREG(CCSIDR_EL1));
+        }


Julien Grall



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