Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

Avi Kivity
2009-06-10T15:24:11Z

Marcelo Tosatti wrote:

>> Ah, you're right.
>>
>> I don't think it's worthwhile to add all this just for debugging. You
>> can add a function that dumps the spte chain as well as the features
>> MSR, and we can decode it by hand when we see it. Disadvantage is more
>> work for us when we hit the bug, but at least that function is reusable
>> in other contexts.
>>
>
> The problem is if someone hits the bug who has no possibility of
> reporting the printks. Nice thing about the WARN_ON's there is you can
> look up kerneloops.org, match the line number to the kernel version,
> and narrow down what bits are wrong (you still need reporter to send
> contents of dmesg for full spte value).
>

Well, we can KERN_WARNING instead of KERN_DEBUG in that printout.

> Also the bit-by-bit validity checks (the inspect_spte function in vmx.c)
> can be used in the mmu audit code (thats the reason for print=0 in the
> callback parameters), so it is reusable in other contexes.
>
> What you dislike is hardcoding the bits checked in C code? Don't worry
> about the level stuff, will be handled next.
>

I just don't want to introduce yet another function-callback pair for
such a small thing. No objection to the code itself.

Printing out the spte hierarchy seems a good idea in other bug contexts,
so at least that function is reusable.

--
error compiling committee.c: too many arguments to function