[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 15/17] x86/shadow: drop SH_type_l2h_pae_shadow
On 22.01.2021 21:02, Tim Deegan wrote: > At 17:31 +0100 on 22 Jan (1611336662), Jan Beulich wrote: >> On 22.01.2021 14:11, Tim Deegan wrote: >>> At 16:10 +0100 on 14 Jan (1610640627), Jan Beulich wrote: >>>> hash_{domain,vcpu}_foreach() have a use each of literal 15. It's not >>>> clear to me what the proper replacement constant would be, as it >>>> doesn't look as if SH_type_monitor_table was meant. >>> >>> Good spot. I think the '<= 15' should be replaced with '< SH_type_unused'. >>> It originally matched the callback arrays all being coded as >>> "static hash_callback_t callbacks[16]". >> >> So are you saying this was off by one then prior to this patch >> (when SH_type_unused was still 17), albeit in apparently a >> benign way? > > Yes - this assertion is just to catch overruns of the callback table, > and so it was OK for its limit to be too low. The new types that were > added since then are never put in the hash table, so don't trigger > this assertion. > >> And the comments in sh_remove_write_access(), >> sh_remove_all_mappings(), sh_remove_shadows(), and >> sh_reset_l3_up_pointers() are then wrong as well, and would >> instead better be like in shadow_audit_tables()? > > Yes, it looks like those comments are also out of date where they > mention 'unused'. For this, which likely will end up being part of ... >> Because of this having been benign (due to none of the callback >> tables specifying non-NULL entries there), wouldn't it make >> sense to dimension the tables by SH_type_max_shadow + 1 only? >> Or would you consider this too risky? > > Yes, I think that would be fine, also changing '<= 15' to > '<= SH_type_max_shadow'. Maybe add a matching > ASSERT(t <= SH_type_max_shadow) in shadow_hash_insert as well? ... this, I'll send a patch for 4.16 going beyond the more immediate one sent, which I'll ask Ian to consider for 4.15 (assuming of course you consider it okay in the first place). Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |