On 21.12.2022 19:58, Andrew Cooper wrote:
> On 21/12/2022 1:27 pm, Jan Beulich wrote:
>> Drop the SH_type_none entry - there are no allocation attempts with
>> this type, and there also shouldn't be any. Adjust the shadow_size()
>> alternative path to match that change. Also generalize two related
>> assertions.
>>
>> While there move the entire table and the respective part of the comment
>> there to hvm.c, resulting in one less #ifdef. In the course of the
>> movement switch to using designated initializers.
>>
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>, although ...
Thanks.
>> --- a/xen/arch/x86/mm/shadow/hvm.c
>> +++ b/xen/arch/x86/mm/shadow/hvm.c
>> @@ -33,6 +33,37 @@
>>
>> #include "private.h"
>>
>> +/*
>> + * This table shows the allocation behaviour of the different modes:
>> + *
>> + * Xen paging 64b 64b 64b
>> + * Guest paging 32b pae 64b
>> + * PV or HVM HVM HVM *
>> + * Shadow paging pae pae 64b
>> + *
>> + * sl1 size 8k 4k 4k
>> + * sl2 size 16k 4k 4k
>> + * sl3 size - - 4k
>> + * sl4 size - - 4k
>> + */
>> +const uint8_t sh_type_to_size[] = {
>> + [SH_type_l1_32_shadow] = 2,
>> + [SH_type_fl1_32_shadow] = 2,
>> + [SH_type_l2_32_shadow] = 4,
>> + [SH_type_l1_pae_shadow] = 1,
>> + [SH_type_fl1_pae_shadow] = 1,
>> + [SH_type_l2_pae_shadow] = 1,
>> + [SH_type_l1_64_shadow] = 1,
>> + [SH_type_fl1_64_shadow] = 1,
>> + [SH_type_l2_64_shadow] = 1,
>> + [SH_type_l2h_64_shadow] = 1,
>> + [SH_type_l3_64_shadow] = 1,
>> + [SH_type_l4_64_shadow] = 1,
>> + [SH_type_p2m_table] = 1,
>> + [SH_type_monitor_table] = 1,
>> + [SH_type_oos_snapshot] = 1,
>
> ... this feels like it's wanting to turn into a (1 + ...) expression. I
> can't see anything that prevents us from reordering the SH_type
> constants, but
>
> 1 + (idx == 1 /* l1 */ || idx == /* fl1 */) + 2 * (idx == 3 /* l2 */);
>
> doesn't obviously simplify further.
But that would then undermine the cases where the function returns 0,
which the two (generalized in this change) assertions actually check
for not having got back. This would also become relevant for the l2h
slot, which - if not right here (see the respective remark) - will
become zero in a subsequent patch when !PV32.
Jan
