|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] x86emul: drop host_and_vcpu_must_have()
On 09.07.2026 13:16, Andrew Cooper wrote: > On 09/07/2026 10:46 am, Jan Beulich wrote: >> With CPU policy dependency logic now properly in place and working, and >> with us never enabling ISA bits in guest policies which the underlying >> hardware doesn't support, the dual checks aren't needed anymore. In fact >> in Misra terms they are dead code. > > This absolutely cannot be called dead code, MISRA or otherwise. > > We have a bunch of logic which is checking for A && B. A and B are > distinct objects in memory, with A always being evaluated, B being > conditionally evaluated, and both having input into the final calculation. > > We believe that in all cases, A has the same value as B, and therefore > the logic can be simplified to just B, but that is a logical leap, not a > transformation that any tooling can make. Well, however much I agree with you, Misra's terminology defines "dead code" thus: "Any operation that is executed but whose removal would not affect program behaviour constitutes dead code." The removal of the cpu_has_* checks here does not affect program behavior, so as per Misra they are "dead code". Eclair would have a hard time spotting such, of course. >> In turn, a number of cpu_has_* can then also go away, when their only user >> was the emulator code. >> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> >> --- >> In my build (with all the other pending work in place) this is a savings >> of about 2k of generated code. >> >> A related question is in how far vcpu_has_*() are useful to retain. We >> could as well access the policy directly, which would even be more >> efficient when a "cp" local variable (or function parameter) is in scope. >> Then x86_emulate()'s instance could also have its __maybe_unused dropped. > > You'd need the test harness able to use a struct cpu_policy in order to > drop vcpu_has_*(). It's doable, but isn't a small change. vcpu_has_*() are already shared between Xen and the harnesses. A CPU policy is also already in use everywhere. So I fear I don't understand what you're trying to point out. >> With the cpu_has_* being dropped I came to wonder why we have e.g. >> cpu_has_lkgs. This particular one was added together with cpu_has_fred, >> yet imo we better wouldn't add such #define-s when they're not going to be >> used. > > Older versions of the FRED series checked for cpu_has_fred && > cpu_has_lkgs, but I dropped the latter on discovering that Xen didn't > need an LKGS instruction at all. > > I think I will need in order to get the guest policies working > correctly, so please don't drop it yet. Sure, I had no immediate plan to do so. Such removal wouldn't belong here. Overall - what does your reply mean for the patch itself? You don't point out any change that would need making, yet you also didn't offer an ack. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |