On 7/23/24 17:22, Bjorn Helgaas wrote:
+ * The st_info struct defines the steering tag returned by the firmware _DSM
+ * method defined in PCI Firmware Spec r3.3, sect 4.6.15 "_DSM to Query Cache
+ * Locality TPH Features"
I don't know what I'm missing, but my copy of the r3.3 spec, dated Jan
20, 2021, doesn't have sec 4.6.15.
According to https://members.pcisig.com/wg/PCI-SIG/document/15470, the
revision has "4.6.15. _DSM to Query Cache Locality TPH Features".
PCI-SIG approved this ECN, but haven't merged it into PCI Firmware
Specification 3.3 yet.
<snip>
+
+/**
+ * pcie_tph_get_st_from_acpi() - Retrieve steering tag for a specific CPU
+ * using platform ACPI _DSM
1) TPH and Steering Tags are not ACPI-specific, even though the only
current mechanism to learn the tags happens to be an ACPI _DSM, so I
think we should omit "acpi" from the name drivers use.
2) The spec doesn't restrict Steering Tags to be for a CPU; it says
"processing resource such as a host processor or system cache
hierarchy ..." But obviously this interface only comprehends an ACPI
CPU ID. Maybe the function name should include "cpu".
How about pcie_tph_get_st_for_cpu() or pcie_tph_retreive_st_for_cpu()?
diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h
index 854677651d81..b12a592f3d49 100644
--- a/include/linux/pci-tph.h
+++ b/include/linux/pci-tph.h
@@ -9,15 +9,27 @@
#ifndef LINUX_PCI_TPH_H
#define LINUX_PCI_TPH_H
+enum tph_mem_type {
+ TPH_MEM_TYPE_VM, /* volatile memory type */
+ TPH_MEM_TYPE_PM /* persistent memory type */
Where does this come from? I don't see "vram" or "volatile" used in
the PCIe spec in this context. Maybe this is from the PCI Firmware
spec?
Yes, this is defined in the ECN mentioned above. Do you have concerns
about defining them here? If we want to remove it, then
pcie_tph_get_st_from_acpi() function can only support one memory type
(e.g. vram). Any advice?
+static inline int pcie_tph_get_st_from_acpi(struct pci_dev *dev, unsigned int cpu_acpi_uid,
+ enum tph_mem_type tag_type, u8 req_enable,
+ u16 *tag)
+{ return false; }
"false" is not "int".
Apparently you want to return "success" in this case, when
CONFIG_PCIE_TPH is not enabled? I suggested leaving this non-exported
for now, which would mean removing this altogether. But if/when we do
export it, I think maybe it should return error so a caller doesn't
assume it succeeded, since *tag will be garbage.
Bjorn
