09.07.24 10:09, Jan Beulich:
On 09.07.2024 07:48, Sergiy Kibrik wrote:
The stub just returns 0 due to implementation of p2m_get_mem_access() for
x86 & ARM expects it to be 0 when altp2m not active or not implemented.
The separate stub is favoured over dynamic check for alt2pm availability
inside regular altp2m_vcpu_idx() because this way we retain the possibility
to later put the whole struct altp2mvcpu under CONFIG_ALTP2M.
The purpose of the change is later to be able to disable altp2m support and
exclude its code from the build completely, when not supported by target
platform (as of now it's implemented for Intel EPT only).
Signed-off-by: Sergiy Kibrik <sergiy_kib...@epam.com>
Yet what doesn't become clear is why 0 is a valid value to return. On the
surface 0 is a valid index when altp2m is enabled. In which case it
couldn't be used (without clarification) when altp2m is disabled.
I looked into it a bit more and found your concerns to be valid --
indeed altp2m_vcpu_idx() should not return valid index when altp2m not
supported. In fact it seems that this routine should not even be called
when altp2m is not active -- all but one call sites check
altp2m_active() first, and there's stub in include/asm-generic/altp2m.h
to block accidental calls to it.
So I think about falling back to v2 of this patch i.e. to guard that one
out of line call in hvm_monitor_check_p2m() but with better patch
description:
https://lore.kernel.org/xen-devel/01767c3f98a88999d4b8ed3ae742ad66a0921ba3.1715761386.git.sergiy_kib...@epam.com/
would that be acceptable ?
-Sergiy