On 14/03/13 16:10, Peter Maydell wrote: > On 13 February 2013 15:47, Marc Zyngier <marc.zyngier@xxxxxxx> wrote: >> Permission fault can be more than just a write fault. Execution >> fault can also occur if the page is flagged with the XN bit. >> >> kvm_decode_fault() returns a value that indicates the fault type >> (read, write, exec). > > (This is only tangentially related to this patch but I wanted > to say it anyway, since we had a conversation about this a few > weeks back and I wanted to record the conclusion.) Indeed, I remember this conversation which was definitely useful. > So this whole code path is really relying on something > subtle which I think it would be good to describe explicitly > in the code (ie in a comment): > > /* When we get a stage 2 Permission fault, there is a possibility > * of a race where the fault_ipa is not in fact correct. (This > * happens because host CPUs do not set the HPFAR on a stage > * 2 Permission fault, and so we have to do a separate VA-to-IPA > * lookup; the stage 1 mapping may change in the window between > * taking the fault to hyp mode and doing the VA-to-IPA lookup.) > * This means that any handling of a stage 2 Permission fault must: > * + be idempotent > * + be safe to do for any IPA > * + be concluded by restarting the faulting guest instruction, > * not by emulating and skipping over it > */ If you don't mind, I'll add this comment to the patch series (I've recently re-spun it and started collecting data points). > I believe all our current permission-fault handling satisfies > these, but if we document this corner case it's less likely > somebody will introduce an incorrect code path later on... Definitely. Thanks for the heads up. M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
