On Thu, Feb 23, 2023 at 11:24 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Thu, Feb 23, 2023, Yu Zhao wrote: > > On Thu, Feb 23, 2023 at 10:09 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > I'll take a look at that series. clear_bit() probably won't cause any > > > > practical damage but is technically wrong because, for example, it can > > > > end up clearing the A-bit in a non-leaf PMD. (cmpxchg will just fail > > > > in this case, obviously.) > > > > > > Eh, not really. By that argument, clearing an A-bit in a huge PTE is also technically > > > wrong because the target gfn may or may not have been accessed. > > > > Sorry, I don't understand. You mean clear_bit() on a huge PTE is > > technically wrong? Yes, that's what I mean. (cmpxchg() on a huge PTE > > is not.) > > > > > The only way for > > > KVM to clear a A-bit in a non-leaf entry is if the entry _was_ a huge PTE, but was > > > replaced between the "is leaf" and the clear_bit(). > > > > I think there is a misunderstanding here. Let me be more specific: > > 1. Clearing the A-bit in a non-leaf entry is technically wrong because > > that's not our intention. > > 2. When we try to clear_bit() on a leaf PMD, it can at the same time > > become a non-leaf PMD, which causes 1) above, and therefore is > > technically wrong. > > 3. I don't think 2) could do any real harm, so no practically no problem. > > 4. cmpxchg() can avoid 2). > > > > Does this make sense? > > I understand what you're saying, but clearing an A-bit on a non-leaf PMD that > _just_ got converted from a leaf PMD is "wrong" if and only if the intented > behavior is nonsensical. Sorry, let me rephrase: 1. Clearing the A-bit in a non-leaf entry is technically wrong because we didn't make sure there is the A-bit there -- the bit we are clearing can be something else. (Yes, we know it's not, but we didn't define this behavior, e.g., a macro to designate that bit for non-leaf entries. Also I didn't check the spec -- does EPT actually support the A-bit in non-leaf entries? My guess is that NPT does.)
