On Wed, 2022-03-23 at 20:25 +0100, Rafael J. Wysocki wrote:
> On Fri, Mar 18, 2022 at 10:30 PM Vishal Verma <vishal.l.verma@xxxxxxxxx> wrote:
> >
> > @@ -385,20 +437,35 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> > ctrl = *mask;
> > *mask |= root->osc_control_set;
> >
> > + if (is_cxl(root)) {
> > + cxl_ctrl = *cxl_mask;
> > + *mask |= root->osc_control_set;
>
> Is this correct?
>
> It appears redundant at least, because the same update has already
> happened above.
Oh good spot - this was a copy-paste error, it needs to be:
*cxl_mask |= root->cxl_osc_control_set;
I've fixed this up and all other comments too for v2.
Thanks for the review!
>
> > + }
> > +
> > /* Need to check the available controls bits before requesting them. */
> > do {
> > - status = acpi_pci_query_osc(root, support, mask);
> > + status = acpi_pci_query_osc(root, support, mask, cxl_support,
> > + cxl_mask);
> > if (ACPI_FAILURE(status))
> > return status;
> > - if (ctrl == *mask)
> > - break;
> > - decode_osc_control(root, "platform does not support",
> > - ctrl & ~(*mask));
> > + if (is_cxl(root)) {
> > + if ((ctrl == *mask) && (cxl_ctrl == *cxl_mask))
> > + break;
> > + decode_cxl_osc_control(root, "platform does not support",
> > + cxl_ctrl & ~(*cxl_mask));
> > + } else {
> > + if (ctrl == *mask)
> > + break;
> > + decode_osc_control(root, "platform does not support",
> > + ctrl & ~(*mask));
> > + }
> > ctrl = *mask;
> > - } while (*mask);
> > + cxl_ctrl = *cxl_mask;
> > + } while (*mask || *cxl_mask);
> >
> > /* No need to request _OSC if the control was already granted. */
> > - if ((root->osc_control_set & ctrl) == ctrl)
> > + if (((root->osc_control_set & ctrl) == ctrl) &&
> > + ((root->cxl_osc_control_set & cxl_ctrl) == cxl_ctrl))
> > return AE_OK;
> >
> > if ((ctrl & req) != req) {
> > @@ -410,11 +477,17 @@ static acpi_status acpi_pci_osc_control_set(acpi_handle handle, u32 *mask, u32 s
> > capbuf[OSC_QUERY_DWORD] = 0;
> > capbuf[OSC_SUPPORT_DWORD] = root->osc_support_set;
> > capbuf[OSC_CONTROL_DWORD] = ctrl;
> > - status = acpi_pci_run_osc(root, capbuf, mask);
> > + if (is_cxl(root)) {
> > + capbuf[OSC_CXL_SUPPORT_DWORD] = root->cxl_osc_support_set;
> > + capbuf[OSC_CXL_CONTROL_DWORD] = cxl_ctrl;
> > + }
> > +
> > + status = acpi_pci_run_osc(root, capbuf, mask, cxl_mask);
> > if (ACPI_FAILURE(status))
> > return status;
> >
> > root->osc_control_set = *mask;
> > + root->cxl_osc_control_set = *cxl_mask;
> > return AE_OK;
> > }
> >
> > @@ -440,6 +513,19 @@ static u32 calculate_support(void)
> > return support;
> > }
> >
> > +static u32 calculate_cxl_support(void)
> > +{
> > + u32 support;
> > +
> > + support = OSC_CXL_2_0_PORT_DEV_REG_ACCESS_SUPPORT;
> > + if (pci_aer_available())
> > + support |= OSC_CXL_PROTOCOL_ERR_REPORTING_SUPPORT;
> > + if (IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
> > + support |= OSC_CXL_NATIVE_HP_SUPPORT;
> > +
> > + return support;
> > +}
> > +
> > static u32 calculate_control(void)
> > {
> > u32 control;
> > @@ -471,6 +557,16 @@ static u32 calculate_control(void)
> > return control;
> > }
> >
> > +static u32 calculate_cxl_control(void)
> > +{
> > + u32 control = 0;
> > +
> > + if (IS_ENABLED(CONFIG_MEMORY_FAILURE))
> > + control |= OSC_CXL_ERROR_REPORTING_CONTROL;
> > +
> > + return control;
> > +}
> > +
> > static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> > {
> > struct acpi_device *device = root->device;
> > @@ -492,6 +588,7 @@ static bool os_control_query_checks(struct acpi_pci_root *root, u32 support)
> > static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> > {
> > u32 support, control = 0, requested = 0;
> > + u32 cxl_support = 0, cxl_control = 0, cxl_requested = 0;
> > acpi_status status;
> > struct acpi_device *device = root->device;
> > acpi_handle handle = device->handle;
> > @@ -515,10 +612,20 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> > if (os_control_query_checks(root, support))
> > requested = control = calculate_control();
> >
> > - status = acpi_pci_osc_control_set(handle, &control, support);
> > + if (is_cxl(root)) {
> > + cxl_support = calculate_cxl_support();
> > + decode_cxl_osc_support(root, "OS supports", cxl_support);
> > + cxl_requested = cxl_control = calculate_cxl_control();
> > + }
> > +
> > + status = acpi_pci_osc_control_set(handle, &control, support,
> > + &cxl_control, cxl_support);
> > if (ACPI_SUCCESS(status)) {
> > if (control)
> > decode_osc_control(root, "OS now controls", control);
> > + if (cxl_control)
> > + decode_cxl_osc_control(root, "OS now controls",
> > + cxl_control);
> >
> > if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
> > /*
> > @@ -547,6 +654,11 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm)
> > decode_osc_control(root, "OS requested", requested);
> > decode_osc_control(root, "platform willing to grant", control);
> > }
> > + if (cxl_control) {
> > + decode_cxl_osc_control(root, "OS requested", cxl_requested);
> > + decode_cxl_osc_control(root, "platform willing to grant",
> > + cxl_control);
> > + }
> >
> > dev_info(&device->dev, "_OSC: platform retains control of PCIe features (%s)\n",
> > acpi_format_exception(status));
> > --
> > 2.35.1
> >
