> 2024年12月4日 03:44，Rafael J. Wysocki <rafael@xxxxxxxxxx> 写道：
>
> Hi,
>
> Sorry for the delay.
>
> On Thu, Nov 21, 2024 at 10:57 PM Miao Wang <shankerwangmiao@xxxxxxxxx> wrote:
>>
>> 2023年11月7日 00:09，Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> 写道：
>>>
>>> +static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
>>> struct acpi_device **adev_p)
>>> {
>>> struct acpi_device *device = acpi_fetch_acpi_dev(handle);
>>> @@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
>>> if (acpi_device_should_be_hidden(handle))
>>> return AE_OK;
>>>
>>> - /* Bail out if there are dependencies. */
>>> - if (acpi_scan_check_dep(handle, check_dep) > 0)
>>> - return AE_CTRL_DEPTH;
>>> + if (first_pass) {
>>> + acpi_mipi_check_crs_csi2(handle);
>>> +
>>> + /* Bail out if there are dependencies. */
>>> + if (acpi_scan_check_dep(handle) > 0) {
>>> + /*
>>> + * The entire CSI-2 connection graph needs to be
>>> + * extracted before any drivers or scan handlers
>>> + * are bound to struct device objects, so scan
>>> + * _CRS CSI-2 resource descriptors for all
>>> + * devices below the current handle.
>>> + */
>>> + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
>>> + ACPI_UINT32_MAX,
>>> + acpi_scan_check_crs_csi2_cb,
>>> + NULL, NULL, NULL);
>>> + return AE_CTRL_DEPTH;
>>> + }
>>> + }
>>>
>>> fallthrough;
>>> case ACPI_TYPE_ANY: /* for ACPI_ROOT_OBJECT */
>>>
>>
>> Hi, I'd like to report some issues caused by this patch. Correct me if I'm wrong
>> since I'm not expert on ACPI. Before this patch, properties of ACPI devices with
>> _DEP relationship declared were evaluated after the initialization of the
>> depending devices, i.e. the execution of handler->attach(). With this patch,
>> _CRS of all the ACPI devices are evaluated to check if the device contains a
>> CSI2 resource, regardless of the declaration of _DEP relationship.
>
> Yes, but the _DEP information is not relevant for whether or not _CRS
> may be evaluated at all.
>
>> The evaluation of _CRS is even before _STA
>
> Not really. _STA has already been evaluated by
> acpi_ns_init_one_device() for all devices at this point.
I quickly scanned the code in acpi_ns_init_one_device(), and it seems
that it an object does not have _INI method, then the _STA will not be
evaluated here.
>
>> and other methods indicating the status of the device.
>
> No, but it doesn't matter, at least by the spec. Had the device been
> disabled, _CRS would have been expected to work anyway:
>
> "If a device is disabled, then _CRS returns a valid resource template
> for the device, but the actual resource assignments in the return byte
> stream are ignored." (ACPI 6.5, Section 6.2.2. _CRS (Current Resource
> Settings)).
>
> That said, adding a device status check before the _CRS evaluation in
> acpi_bus_check_add() is not inconceivable.
>
>> This has caused some issues in certain scenarios, where the DSDT contains legacy
>> devices, which requires reading IO ports to form the result of its _CRS. An
>> example of such a legacy device is declared in the DSDT is as below:
>>
>> Device (LPT)
>> {
>> Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)
>> Name (_UID, 0x02) // _UID: Unique ID
>> Name (_DDN, "LPT1") // _DDN: DOS Device Name
>> Name (_DEP, Package (0x01) // _DEP: Dependencies
>> {
>> \_SB.PCI0
>> })
>> OperationRegion (ITE1, SystemIO, 0x2E, 0x02)
>> Field (ITE1, ByteAcc, NoLock, Preserve) {INDX, 8, DATA, 8}
>> IndexField (INDX, DATA, ByteAcc, NoLock, Preserve)
>> {
>> // Omitting some declarations of fields
>> IOAH, 8,
>> IOAL, 8,
>> }
>> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings
>> {
>> Name (BUF0, ResourceTemplate ()
>> {
>> IO (Decode16,
>> 0x0378, // Range Minimum
>> 0x0378, // Range Maximum
>> 0x01, // Alignment
>> 0x08, // Length
>> _Y01)
>> IRQ (Level, ActiveLow, Exclusive, _Y02)
>> {7}
>> })
>> CreateByteField (BUF0, \LPT._CRS._Y01._MIN, IOLO)
>> CreateByteField (BUF0, 0x03, IOHI)
>> CreateByteField (BUF0, \LPT._CRS._Y01._MAX, IORL)
>> CreateByteField (BUF0, 0x05, IORH)
>> CreateByteField (BUF0, \LPT._CRS._Y01._LEN, LEN1)
>> CreateByteField (BUF0, \LPT._CRS._Y02._INT, IRQL)
>>
>> IOLO = IOAL /* \LPT_.IOAL */
>> IORL = IOAL /* \LPT_.IOAL */
>> IOHI = IOAH /* \LPT_.IOAH */
>> IORH = IOAH /* \LPT_.IOAH */
>> // Omitting some assignments and calculations
>>
>> Return (BUF0)
>> }
>> }
>>
>> On non-x86 platforms, IO ports are implemented using MMIO. The memory is
>> remapped in the initialization of the PCI root, using its QWordIO resource
>> declared in its _CRS, in acpi_pci_root_remap_iospace(). Before the
>> initialization of the PCI root, reads and writes of the IO ports will result in
>> accessing memory region which is not mapped.
>
> That's not really straightforward.
>
>> Before this patch, adding a _DEP of Package(){PCI0} to these legacy devices will
>> postpone the initialization of these devices after the PCI root, solving the
>> above problem. After this patch, however, the evaluation of _CRS regardless of
>> _DEP declarations causes accessing IO ports before they are ready.
>
> Well, before this patch, Linux simply did not have to evaluate _CRS
> during device discovery.
>
> Nowhere in the spec it is stated that _CRS cannot be evaluated for a
> given device before enumerating all devices listed by its _DEP object.
> As it is currently defined, _DEP is only about operation region
> dependencies, not even about general device enumeration ordering (even
> though it is used for enforcing specific enumeration ordering in the
> field because OSes tend to enumerate devices in the order following
> from _DEP as a matter of fact).
>
>> I've checked the ACPI spec, and it states that ``OSPM must guarantee that the
>> following operation regions are always accessible: SystemIO operation regions"
>
> Which to me is clearly at odds with the SystemIO implementation
> description above.
>
>> and ``Since the region types above are permanently available, no _REG methods
>> are required, nor will OSPM evaluate any _REG methods that appear in the same
>> scope as the operation region declaration(s) of these types". It seems that
>> from the view of the AML codes in the DSDT, it can never know when the IO ports
>> are ready, and it is impossible for Linux on non-x86 platforms to ensure IO
>> ports are always accessible.
>
> They aren't always accessible anyhow. They become accessible after
> enumerating the PCI host bridge and they aren't accessible earlier.
>
>> I wonder if there would be a better solution to this problem.
>
> Well, it is a bit unfortunate that it took 6 kernel release cycles to
> realize that there was a problem. Had this been reported earlier,
> there would have been more options on the table.
Yeah, since it is rare to keep a legacy device on non-X86 platforms,
and as a result, this issue is not discovered until now.
>
> At this point, the functionality related to CSI-2 connection graphs
> simply needs to be there, the only option for avoiding _CRS evaluation
> in acpi_bus_check_add() would be to somehow defer the enumeration of
> all devices using CSI-2 connections until the host bridge is
> enumerated. Alternatively, the enumeration of the PCI host bridge
> might be pushed forward, but that would require addressing at least a
> few enumeration ordering issues. None of these would be a small
> change and backporting any of that would have been a considerable
> effort.
>
> One of the things worth considering is whether or not CSI-2 is
> relevant for the architectures that rely on the old behavior related
> to _DEP. If not at all, a Kconfig option could be added to disable
> CSI-2 graphs enumeration on those architectures and that would make
> the problem go away.
I'm not familiar with ACPI. However, in my personal point of view, it
seems that the root cause of the issue is in the ACPI standard, which
assumes SystemIO operation regions are always available, which is not
the case on non-X86 platforms. Also, there is no mechanism for the DSDT
code to know whether SystemIO operation regions are available, and it
is thus impossible to return ``a valid resource template" in the _CRS
method when SystemIO operation regions are unavailable. So it is not
the architectures that want to rely on the old behavior related to
_DEP, but all non-x86 architectures with ACPI support may have no
choice but to rely on this to postpone possible IO port operations after
the initialization of the PCI host bridge. However, on non-x86
architectures, there might be limited number of machines with devices
using legacy IO operations, so this issue is currently not so significant.
The documents for CSI-2 give me an impression that it is related to a
certain kind of camera, so I don't think it is good to remove this for
specific architectures.
> Also I'm wondering if the firmware could be updated to survive a mere
> evaluation of _CRS if the hardware is not ready.
If we only consider the current ACPI standard, I cannot come up with
a solution on the firmware side, as discussed before. But utilizing
(or abusing?) the current Linux implementation details, I managed to
work this around in this way:
Device (LPT)
{
//...
Name (STAD, Zero)
Name (_DEP, Package (0x01)
{
\_SB.PCI0
})
Method (_STA, 0, NotSerialized)
{
STAD = One
// other codes
}
Method (_CRS, 0, NotSerialized)
{
Name (BUF0,/*....*/)
// Initialize BUF0
If (STAD)
{
// do actual IO operations and fill BUF0 with actual
// numbers
}
Return (BUF0)
}
}
This work around is because I noticed the _STA method is called after
the depending PCI0 is initialized, so set a flag in the _STA method to
mark the availability of the IO port operations.
Maybe we can prioritize the initialization of the PCI host bridge to
fully eliminate this issue?
Cheers,
Miao Wang
