On Wed, Mar 24, 2021, Krish Sadhukhan wrote:
> According to section "Canonicalization and Consistency Checks" in APM vol 2,
> the following guest state is illegal:
>
> "The MSR or IOIO intercept tables extend to a physical address that
> is greater than or equal to the maximum supported physical address."
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> x86/svm_tests.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/x86/svm_tests.c b/x86/svm_tests.c
> index 29a0b59..70442d2 100644
> --- a/x86/svm_tests.c
> +++ b/x86/svm_tests.c
> @@ -2304,6 +2304,33 @@ static void test_dr(void)
> vmcb->save.dr7 = dr_saved;
> }
>
> +/*
> + * If the MSR or IOIO intercept table extends to a physical address that
> + * is greater than or equal to the maximum supported physical address, the
> + * guest state is illegal.
> + *
> + * [ APM vol 2]
> + */
> +static void test_msrpm_iopm_bitmap_addrs(void)
> +{
> + u64 addr_spill_beyond_ram =
FWIW, it's not "beyond ram", it's beyond the legal physical address space. E.g.
the address can point at stuff other than RAM and be perfectly legal from a
consistency check perspective.
> + (u64)(((u64)1 << cpuid_maxphyaddr()) - 4096);
It'd be nice to also check a straight legal address, and an address that
straddles the high address => 0.
> +
> + /* MSR bitmap address */
> + vmcb->control.intercept |= 1ULL << INTERCEPT_MSR_PROT;
> + vmcb->control.msrpm_base_pa = addr_spill_beyond_ram;
> + report(svm_vmrun() == SVM_EXIT_ERR, "Test MSRPM address: %lx",
> + addr_spill_beyond_ram);
> + vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
> +
> + /* MSR bitmap address */
> + vmcb->control.intercept |= 1ULL << INTERCEPT_IOIO_PROT;
> + vmcb->control.msrpm_base_pa = addr_spill_beyond_ram;
Wrong bitmap.
> + report(svm_vmrun() == SVM_EXIT_ERR, "Test IOPM address: %lx",
> + addr_spill_beyond_ram);
> + vmcb->control.intercept &= ~(1ULL << INTERCEPT_IOIO_PROT);
The control should be save/restored, assuming the intercepts were clear will
cause reproducibility issues for other tests.
> +}
> +
> static void svm_guest_state_test(void)
> {
> test_set_guest(basic_guest_main);
> @@ -2313,6 +2340,7 @@ static void svm_guest_state_test(void)
> test_cr3();
> test_cr4();
> test_dr();
> + test_msrpm_iopm_bitmap_addrs();
> }
>
>
> --
> 2.27.0
>
