Re: [Xen-devel] [PATCH for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

[Andrew Cooper]

On 16/05/18 09:14, Jan Beulich wrote:
 On 15.05.18 at 19:54,  wrote:
>> Also, I don't see any link between the change and the commit message.  With
>> the microcode installed, STIBP and IBPB are already visible to dom0.
> They reportedly weren't (and I was able to confirm that), and given this
> original (prior to that change) code
>
> }
> }
> else
> b = c = 0;
> a = d = 0;
>
> I also can't see how IBRSB and STIBP could have been visible. I agree I
> had wrongly extended that to IBPB.
>
>> The only required adjustment is to force STIBP == IBRSB, which must be done
>> after applying the pv_featureset[] mask to the toolstack's choice of value.
> I can see how I've got that part wrong from a leveling perspective (I was
> really too focused on Dom0 back then), but I don't see how reporting IBPB
> when IBRSB is available in hardware (implying IBPB itself isn't) would work
> with your change in place.

I've submitted v2 with an updated commit message, now I understand where
the dom0 comment came from.

>
> I'm also not convinced assimilating Sergey's original change into this one is
> appropriate - raw_featureset[] isn't used for anything except the sysctl.

You regressed a feature with an incorrect backport, in a way which
directly impacts a tool which administrators will use to see if they've
got the microcode applied properly.

It was broken in a security patch, therefore it is going to get fixed.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

[Jan Beulich]

>>> On 15.05.18 at 19:54,  wrote:
> Also, I don't see any link between the change and the commit message.  With
> the microcode installed, STIBP and IBPB are already visible to dom0.

They reportedly weren't (and I was able to confirm that), and given this
original (prior to that change) code

}
}
else
b = c = 0;
a = d = 0;

I also can't see how IBRSB and STIBP could have been visible. I agree I
had wrongly extended that to IBPB.

> The only required adjustment is to force STIBP == IBRSB, which must be done
> after applying the pv_featureset[] mask to the toolstack's choice of value.

I can see how I've got that part wrong from a leveling perspective (I was
really too focused on Dom0 back then), but I don't see how reporting IBPB
when IBRSB is available in hardware (implying IBPB itself isn't) would work
with your change in place.

I'm also not convinced assimilating Sergey's original change into this one is
appropriate - raw_featureset[] isn't used for anything except the sysctl.

Jan



___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel