Re: Problems in PV dom0 on recent x86 hardware

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.07.2024 08:36, Jürgen Groß wrote:
> On 09.07.24 08:24, Jan Beulich wrote:
>> On 08.07.2024 23:30, Jason Andryuk wrote:
>>>   From the backtrace, it looks like the immediate case is just trying to
>>> read a 4-byte version:
>>>
>>>   >>>> [   44.575541]  ucsi_acpi_dsm+0x53/0x80
>>>   >>>> [   44.575546]  ucsi_acpi_read+0x2e/0x60
>>>   >>>> [   44.575550]  ucsi_register+0x24/0xa0
>>>   >>>> [   44.575555]  ucsi_acpi_probe+0x162/0x1e3
>>>
>>> int ucsi_register(struct ucsi *ucsi)
>>> {
>>>           int ret;
>>>
>>>           ret = ucsi->ops->read(ucsi, UCSI_VERSION, &ucsi->version,
>>>                                 sizeof(ucsi->version));
>>>
>>> ->read being ucsi_acpi_read()
>>>
>>> However, the driver also appears write to adjacent addresses.
>>
>> There are also corresponding write functions in the driver, yes, but
>> ucsi_acpi_async_write() (used directly or indirectly) similarly calls
>> ucsi_acpi_dsm(), which wires through to acpi_evaluate_dsm(). That's
>> ACPI object evaluation, which isn't obvious without seeing the
>> involved AML whether it might write said memory region.
> 
> I guess an ACPI dump would help here?

Perhaps, yes.

>> The writing
>> done in the write function(s) looks to be
>>
>>      memcpy(ua->base + offset, val, val_len);
>>
>> with their read counterpart being
>>
>>      memcpy(val, ua->base + offset, val_len);
>>
>> where ua->base may well be an entirely different address (looks like
>> it's the first of the BARs as per ucsi_acpi_probe()).
> 
> According to the lspci -v output there are no BARs in the MSI space:
> 
> 66:00.6 USB controller: Advanced Micro Devices, Inc. [AMD] Pink Sardine 
> USB4/Thunderbolt NHI controller #2 (prog-if 40 [USB4 Host Interface])
>       Subsystem: Lenovo Device 50d9
>       Flags: bus master, fast devsel, latency 0, IRQ 71
>       Memory at 78a00000 (64-bit, non-prefetchable) [size=512K]
>       Capabilities: [48] Vendor Specific Information: Len=08 <?>
>       Capabilities: [50] Power Management version 3
>       Capabilities: [64] Express Endpoint, MSI 00
>       Capabilities: [a0] MSI: Enable- Count=1/16 Maskable- 64bit+
>       Capabilities: [c0] MSI-X: Enable+ Count=16 Masked-
>       Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010 
> <?>
>       Capabilities: [2a0] Access Control Services
>       Kernel driver in use: thunderbolt
>       Kernel modules: thunderbolt

Right, this matches what I was vaguely guessing from reading the code in
the driver. My present understanding is that the object evaluation
actually triggers the read/write operation to produce/consume data inside
that single BAR's space.

>> If acpi_evaluate_dsm() would only ever read the region, an option (if
>> all else fails) might be to similarly (to what we do for IO-APICs)
>> permit read accesses / mappings (by inserting the range into
>> mmio_ro_ranges). Yet of course first we need to better understand
>> what's actually going on here.
> 
> As the mapping is currently trying to allow write access, too, the kernel
> would need some modification as well.

Not really, no. It would be better if the kernel didn't ask for write
access, but get_page_from_l1e() simply tells its caller to remove the
W bit from the PTE in case a page is recorded in mmio_ro_ranges. That's
also why for the IO-APIC case we got away without needing to alter the
kernel (which would likely be pretty ugly, as acpi_os_map_iomem() sits
very far away from the place where we would have a way to know that a
mapping is sufficient to be r/o; the function itself takes only
address and size right now, no permissions or cachability or anything).

Jan