On Sat, Nov 12, 2022, Gavin Shan wrote: > Hi Marc, > > On 11/11/22 11:19 PM, Marc Zyngier wrote: > > On Thu, 10 Nov 2022 23:47:41 +0000, > > Gavin Shan <gshan@xxxxxxxxxx> wrote: > > But that I don't get. Or rather, I don't get the commit message that > > matches this hunk. Do we want to catch the case where all of the > > following are true: > > > > - we don't have a vcpu, > > - we're allowed to log non-vcpu dirtying > > - we *only* have the ring? As written, no, because the resulting WARN will be user-triggerable. As mentioned earlier in the thread[*], if ARM rejects KVM_DEV_ARM_ITS_SAVE_TABLES when dirty logging is enabled with a bitmap, then this code can WARN. > > If so, can we please capture that in the commit message? > > > > Nice catch! This particular case needs to be warned explicitly. Without > the patch, kernel crash is triggered. With this patch applied, the error > or warning is dropped silently. We either check memslot->dirty_bitmap > in mark_page_dirty_in_slot(), or check it in kvm_arch_allow_write_without_running_vcpu(). > I personally the later one. Let me post a formal patch on top of your > 'next' branch where the commit log will be improved accordingly. As above, a full WARN is not a viable option unless ARM commits to rejecting KVM_DEV_ARM_ITS_SAVE_TABLES in this scenario. IMO, either reject the ITS save or silently ignore the goof. Adding a pr_warn_ratelimited() to alert the user that they shot themselves in the foot after the fact seems rather pointless if KVM could have prevented the self-inflicted wound in the first place. [*] https://lore.kernel.org/all/Y20q3lq5oc2gAqr+@xxxxxxxxxx
