Re: [patch 1/4] KVM: MMU audit: update count_writable_mappings / count_rmaps

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Jun 08, 2009 at 12:24:08PM +0300, Avi Kivity wrote:
>> +static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp,
>> +			    inspect_spte_fn fn)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < PT64_ENT_PER_PAGE; ++i) {
>> +		u64 ent = sp->spt[i];
>> +
>> +		if (is_shadow_present_pte(ent)) {
>> +			if (sp->role.level > 1) {
>>   
>
> I think this is broken wrt large pages.  We should recurse if role.level  
> > 1 or the G bit is set.

Yes, fixed. Plan to add largepages validity checks later.

> Semi-related: we should set up a new exit code to halt the VM so it can  
> be inspected, otherwise all those printks and dump_stack()s will quickly  
> overwhelm the logging facilities.

Can you clarify on the halt exit code?

Because for other exit codes which similar behaviour is wanted, say,
unhandled vm exit, the policy can be handled in userspace (and the
decision to halt or not seems better suited to happen there). So perhaps
KVM_EXIT_MMU_AUDIT_FAILED?

I wondered before whether it would be good to stop auditing on the first
error, but gave up on the idea.