Hi Julien, > On 31 Aug 2021, at 15:47, Julien Grall <[email protected]> wrote: > > > > On 31/08/2021 14:17, Bertrand Marquis wrote: >> Hi Julien, > > Hi Bertrand, > >>> On 27 Aug 2021, at 16:05, Julien Grall <[email protected]> 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. > > [...] > >> If we get interrupted, someone could program CSSELR differently and the next >> read >> 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. I could just remove the comment, I added it as information, but if you think it is misleading no problem. Anyway as we will park this for now no need to discuss that further. Cheers Bertrand > >> 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)); >>>> + } > > Cheers, > > -- > Julien Grall
