Re: [PATCH] x86/pci: Correct ECS handling with CF8/CFC emulation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.04.2023 17:04, Roger Pau Monné wrote:
> On Tue, Apr 04, 2023 at 02:27:36PM +0100, Andrew Cooper wrote:
>> On 03/04/2023 2:26 pm, Roger Pau Monné wrote:
>>> On Mon, Apr 03, 2023 at 11:16:52AM +0100, Andrew Cooper wrote:
>>>> On 03/04/2023 9:57 am, Roger Pau Monné wrote:
>>>>>> @@ -1104,6 +1092,11 @@ static int cf_check write_msr(
>>>>>>          if ( !is_hwdom_pinned_vcpu(curr) )
>>>>>>              return X86EMUL_OKAY;
>>>>>>          if ( (rdmsr_safe(MSR_AMD64_NB_CFG, temp) != 0) ||
>>>>>> +             /*
>>>>>> +              * TODO: this is broken.  What happens when dom0 is pinned 
>>>>>> but
>>>>>> +              * can't see the full system?  CF8_EXT probably ought to 
>>>>>> be a
>>>>>> +              * Xen-owned setting, and made symmetric across the system.
>>>>>> +              */
>>>>> I would assume CF8_EXT would be symmetric across the system, specially
>>>>> given that it seems to be phased out and only used in older AMD
>>>>> families that where all symmetric?
>>>> The CF8_EXT bit has been phased out.  The IO ECS functionality still 
>>>> exists.
>>>>
>>>> But yes, the more I think about letting dom0 play with this, the more I
>>>> think it is a fundamentally broken idea...  I bet it was from the very
>>>> early AMD Fam10h days where dom0 knew how to turn it on, and Xen was
>>>> trying to pretend it didn't have to touch any PCI devices.
>>> It seems to me Xen should set CF8_EXT on all threads (when available)
>>> and expose it to dom0, so that accesses using pci_{conf,write}_read()
>>> work as expected?
>>
>> It's per northbridge in the system, not per thread.  Hence needing the
>> spinlock protecting the CF8/CFC IO port pair access, and why MMCFG is
>> strictly preferable.
> 
> So just setting CF8_EXT_ENABLE on MSR_AMD64_NB_CFG by the BSP should
> be enough to have it enabled?  I expect all other threads will see the
> bit as set in the MSR then.

No, it's one bit per socket iirc.

Jan