On Thu, Apr 22, 2021 at 01:05:23PM +0200, Jan Beulich wrote: > On 22.04.2021 12:56, Roger Pau Monné wrote: > > On Thu, Apr 22, 2021 at 12:48:36PM +0200, Jan Beulich wrote: > >> On 22.04.2021 12:34, Roger Pau Monné wrote: > >>> On Thu, Apr 22, 2021 at 11:58:45AM +0200, Jan Beulich wrote: > >>>> On 22.04.2021 11:42, Roger Pau Monné wrote: > >>>>> On Wed, Apr 14, 2021 at 03:49:02PM +0200, Jan Beulich wrote: > >>>>>> On 13.04.2021 16:01, Roger Pau Monne wrote: > >>>>>>> @@ -944,3 +945,130 @@ bool xc_cpu_policy_is_compatible(xc_interface > >>>>>>> *xch, const xc_cpu_policy_t host, > >>>>>>> > >>>>>>> return false; > >>>>>>> } > >>>>>>> + > >>>>>>> +static uint64_t level_msr(unsigned int index, uint64_t val1, > >>>>>>> uint64_t val2) > >>>>>>> +{ > >>>>>>> + uint64_t val = val1 & val2;; > >>>>>> > >>>>>> For arbitrary MSRs this isn't going to do any good. If only very > >>>>>> specific MSRs are assumed to make it here, I think this wants > >>>>>> commenting on. > >>>>> > >>>>> I've added: "MSRs passed to level_msr are expected to be bitmaps of > >>>>> features" > >>>> > >>>> How does such a comment help? I.e. how does the caller tell which MSRs > >>>> to pass here and which to deal with anyother way? > >>> > >>> All MSRs should be passed to level_msr, but it's handling logic would > >>> need to be expanded to support MSRs that are not feature bitmaps. > >>> > >>> It might be best to restore the previous switch and handle each MSR > >>> specifically? > >> > >> I think so, yes. We need to be very careful with what a possible > >> default case does there, though. > > > > Maybe it would be better to handle level_msr in a way similar to > > level_leaf: return true/false to notice whether the MSR should be > > added to the resulting compatible policy? > > > > And then make the default case in level_msr just return false in order > > to prevent adding MSRs not properly leveled to the policy? > > I'm afraid I'm not clear about the implications. What again is the > (planned?) final effect of an MSR not getting added there?
Adding the MSR with a 0 value will zero out any previous value on the 'out' policy, while not adding it would leave the previous value there given the current code in xc_cpu_policy_calc_compatible added by this patch. I would expect callers of xc_cpu_policy_calc_compatible to pass a zeroed 'out' policy, so I think the end result should be the same. Maybe I should also clear 'out' in xc_cpu_policy_calc_compatible, at which point both options will get the same exact result. Thanks, Roger.