Re: [PATCH 4/7] x86: detect PIC aliasing on ports other than 0x[2A][01]

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.10.2023 17:19, Roger Pau Monné wrote:
> On Thu, Oct 26, 2023 at 05:07:18PM +0200, Jan Beulich wrote:
>> On 26.10.2023 15:24, Roger Pau Monné wrote:
>>> On Thu, Oct 26, 2023 at 11:03:42AM +0200, Jan Beulich wrote:
>>>> On 26.10.2023 10:34, Roger Pau Monné wrote:
>>>>> On Thu, May 11, 2023 at 02:06:46PM +0200, Jan Beulich wrote:
>>>>>> ... in order to also deny Dom0 access through the alias ports. Without
>>>>>> this it is only giving the impression of denying access to both PICs.
>>>>>> Unlike for CMOS/RTC, do detection very early, to avoid disturbing normal
>>>>>> operation later on.
>>>>>>
>>>>>> Like for CMOS/RTC a fundamental assumption of the probing is that reads
>>>>>> from the probed alias port won't have side effects in case it does not
>>>>>> alias the respective PIC's one.
>>>>>
>>>>> I'm slightly concerned about this probing.
>>>>>
>>>>> Also I'm unsure we can fully isolate the hardware domain like this.
>>>>> Preventing access to the non-aliased ports is IMO helpful for domains
>>>>> to realize the PIT is not available, but in any case such accesses
>>>>> shouldn't happen in the first place, as dom0 must be modified to run
>>>>> in such mode.
>>>>
>>>> That's true for PV Dom0, but not necessarily for PVH. Plus by denying
>>>> access to the aliases we also guard against bugs in Dom0, if some
>>>> component thinks there's something else at those ports (as they
>>>> indeed were used for other purposes by various vendors).
>>>
>>> I think it would be safe to add a command line option to disable the
>>> probing, as we would at least like to avoid it in pvshim mode.  Maybe
>>> ut would be interesting to make it a Kconfig option so that exclusive
>>> pvshim Kconfig can avoid all this?
>>>
>>> Otherwise it will just make booting the pvshim slower.
>>
>> I've taken note to introduce such an option (not sure yet whether just
>> cmdline or also Kconfig). Still
>> - Shouldn't we already be bypassing related init logic in shim mode?
> 
> Not sure what we bypass in pvshim mode, would be good to double
> check.
> 
>> - A Kconfig option interfacing with PV_SHIM_EXCLUSIVE will collide with
>>   my patch inverting that option's sense (and renaming it), so it would
>>   be nice to have that sorted/accepted first (see
>>   https://lists.xen.org/archives/html/xen-devel/2023-03/msg00040.html).
> 
> It being Andrew the one that made the request, I would like to get his
> opinion on it.  UNCONSTRAINED does seem a bit weird.

I agree that the name is odd; I couldn't think of any better one (and
this already is the result of 3 or 4 rounds of renaming). I'll be more
than happy to consider other naming suggestions. The main issue with the
present option is, just to re-state it here, that we have grown negative
dependencies on it, which is a problem.

However, meanwhile I've realized that we don't really want to tie anything
here to PV_SHIM_EXCLUSIVE, at least not directly. What we care about is
whether we _actually_ run in shim mode, i.e. also when a full-fledged
hypervisor is in use. My plan is now to have said new command line option,
which - if not specified on the command line - I'd default to !pv_shim.

> Maybe the issue is that PV_SHIM_EXCLUSIVE shouldn't have been a
> Kconfig option in the first place, and instead a specific Kconfig
> config file?
> 
> Maybe it's not possible to achieve the same using just a Kconfig
> config file.

I'm afraid I don't understand what you mean by "Kconfig config file". It
can't really be just another .../Kconfig file somewhere in the tree, as
it doesn't really matter where an option like this would be defined.

Jan