On 23.01.2023 13:30, Andrew Cooper wrote: > On 23/01/2023 10:47 am, Jan Beulich wrote: >> On 23.01.2023 11:43, Andrew Cooper wrote: >>> On 23/01/2023 8:12 am, Jan Beulich wrote: >>>> While the table is used only when HVM=y, the table entry of course needs >>>> to be properly populated when also PV32=y. Fully removing the table >>>> entry we therefore wrong. >>>> >>>> Fixes: 1894049fa283 ("x86/shadow: L2H shadow type is PV32-only") >>>> Signed-off-by: Jan Beulich <jbeul...@suse.com> >>> Erm, why? >>> >>> The safety justification for the original patch was that this is HVM >>> only code. And it really is HVM only code - it's genuinely compiled out >>> for !HVM builds. >> Right, and we have logic taking care of the !HVM case. But that same >> logic uses this "HVM-only" table when HVM=y also for all PV types. > > Ok - this is what needs fixing then. > > This is a layering violation which has successfully tricked you into > making a buggy patch. > > I'm unwilling to bet this will be the final time either... "this file > is HVM-only, therefore no PV paths enter it" is a reasonable > expectation, and should be true.
Nice abstract consideration, but would mind pointing out how you envision shadow_size() to look like meeting your constraints _and_ meeting my demand of no excess #ifdef-ary? The way I'm reading your reply is that you ask to special case L2H _right in_ shadow_size(). Then again see also my remark in the original (now known faulty) patch regarding such special casing. I could of course follow that route, regardless of HVM (i.e. unlike said there not just for the #else part) ... Jan