[L] Change in coreboot[main]: acpi: Add acpigen_write_OSC_pci_domain

[Nico Huber (Code Review)]

Attention is currently required from: Arthur Heymans, Christian Walter, Cliff Huang, Johnny Lin, Jonathan Zhang, Lance Zhao, Paul Menzel, Shuo Liu, Tim Chu, Tim Wawrzynczak.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81375?usp=email )

Change subject: acpi: Add acpigen_write_OSC_pci_domain
......................................................................

Patch Set 13:

(4 comments)

Commit Message:

https://review.coreboot.org/c/coreboot/+/81375/comment/21e102d3_39766ac8 :
PS12, Line 9: dynamic
> This is mainly for xeon-sp codes that one set of codes needs to handle multiple SKU or silicon, wher […]

Sure, you cannot easily do it with static DSDT only. But most of the implementation
is boilerplate which could be shared. e.g. one `Method (POSC)` for PCI and one
`Method (COSC)` CXL that could be called from the individual device nodes' `_OSC`.

Either way we decide to do it, it's much easier to decide on the code in ASL form
first, without the generation code. The latter is just harder to review.

File src/acpi/acpigen_pci.c:

https://review.coreboot.org/c/coreboot/+/81375/comment/d774ce7c_9aca3f6b :
PS12, Line 127: 
              : 	if (is_cxl_domain) {
              : 	/*
              : 	 *	If ((Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) &&
              : 	 *		(Arg0 != ToUUID (CXL_HOST_BRIDGE_OSC_UUID)))
              : 	 */
              : 		acpigen_write_if();
              : 		acpigen_emit_byte(LAND_OP);
              : 		acpigen_emit_byte(LNOT_OP);
              : 		acpigen_emit_byte(LEQUAL_OP);
              : 		acpigen_emit_byte(ARG0_OP);
              : 		acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID);
              : 		acpigen_emit_byte(LNOT_OP);
              : 		acpigen_emit_byte(LEQUAL_OP);
              : 		acpigen_emit_byte(ARG0_OP);
              : 		acpigen_write_uuid(CXL_HOST_BRIDGE_OSC_UUID);
              : 	} else {
              : 	/*
              : 	 *	If (Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID))
              : 	 */
              : 		acpigen_write_if();
              : 		acpigen_emit_byte(LNOT_OP);
              : 		acpigen_emit_byte(LEQUAL_OP);
              : 		acpigen_emit_byte(ARG0_OP);
              : 		acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID);
              : 	}
              : 
              :
> For PCIe domain, only PCIe UUID can be handled; but for CXL domain, it can handle both. […]

I don't understand. Currently this looks like:
```
CXL:
    if (!a && !b)
PCI:
    if (!a)
common:
        unrecognized;

check-rev;

PCI:
    if (a)
        ...
CXL:
    if (a)
        ...
    if (b)
        ...
```
while we could do (if I'm not mistaken):
```
check-rev;

if (a)
    ...

CXL-only:
    else if (b)
        ...

else
    unrecognized;
```
Which seems a lot leaner and easier to follow. However the whole thing
depends on the interpretation what to do with the PCI dwords when the
CXL UUID gets called. So let's postpone this for the moment.

https://review.coreboot.org/c/coreboot/+/81375/comment/d0c8d194_c98c367d :
PS12, Line 317: 	acpigen_write_create_dword_field(LOCAL7_OP, 0x10, "OTRC");
> The current assumption is, for a CXL domain

Where does this assumption come from. The code that is removed in the
follow-up didn't seem to make this assumption.

Where is the specification for the CXL part?

https://review.coreboot.org/c/coreboot/+/81375/comment/c044fabe_a79d2c45 :
PS12, Line 348: __weak unsigned long get_granted_pci_features(const struct device *domain)
              : {
              : 	return 0;
              : }
              : 
              : __weak unsigned long get_granted_cxl_features(const struct device *domain)
              : {
              : 	return 0;
              : }
> Please refer to https://review.coreboot.org/c/coreboot/+/81377 where the code is called and tested. […]

Let me rephrase: What if these are not overridden? Does a `return 0`
implementation result in reasonable behavior? Would it differ from not
having an _OSC at all?

Also, CB:81377 does not override get_granted_cxl_features() which makes
_OSC always mask all features. This is very different from what the
removed code does.