Re: [PATCH 2/2] acpi/pci_root: negotiate CXL _OSC

["Verma, Vishal L" <vishal.l.verma@intel.com>]

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@intel.com> 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
> >