| Message ID | 20221115031244.1667093-1-LeoLiu-oc@zhaoxin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Parse the PCIe AER and set to relevant registers | expand |
Since this patch has nothing to do with ACPI, update subject line to: PCI: Add PCIe to PCI/PCI-X Bridge AER fields On Tue, Nov 15, 2022 at 11:12:44AM +0800, LeoLiu-oc wrote: > From: leoliu-oc <leoliu-oc@zhaoxin.com> > > Define secondary uncorrectable error mask register, secondary > uncorrectable error severity register and secondary error capabilities and > control register bits in AER capability for PCIe to PCI/PCI-X Bridge. > Please refer to PCIe to PCI/PCI-X Bridge Specification, sec 5.2.3.2, > 5.2.3.3 and 5.2.3.4. Capitalize register names to match the spec usage. > Signed-off-by: leoliu-oc <leoliu-oc@zhaoxin.com> Assuming this goes along with a patch series that adds uses of these definitions: Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > include/uapi/linux/pci_regs.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index 57b8e2ffb1dd..37f3baa336d7 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -799,6 +799,11 @@ > #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */ > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ > > +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ > +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */ > +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */ > +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */ Please squash these right up next to the other PCI_ERR_* definitions so it's obvious that they overlap PCI_ERR_ROOT_STATUS and PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both), e.g., #define PCI_ERR_ROOT_STATUS 0x30 #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */ ... #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */ #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */ #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */ > /* Virtual Channel */ > #define PCI_VC_PORT_CAP1 0x04 > #define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */ > -- > 2.20.1 >
在 2023/4/8 7:22, Bjorn Helgaas 写道: > Since this patch has nothing to do with ACPI, update subject line to: > > PCI: Add PCIe to PCI/PCI-X Bridge AER fields > Your description is more reasonable and I will update the header of this patch later. Yours sincerely, Leoliu-oc > On Tue, Nov 15, 2022 at 11:12:44AM +0800, LeoLiu-oc wrote: >> From: leoliu-oc <leoliu-oc@zhaoxin.com> >> >> Define secondary uncorrectable error mask register, secondary >> uncorrectable error severity register and secondary error capabilities and >> control register bits in AER capability for PCIe to PCI/PCI-X Bridge. >> Please refer to PCIe to PCI/PCI-X Bridge Specification, sec 5.2.3.2, >> 5.2.3.3 and 5.2.3.4. > > Capitalize register names to match the spec usage. > Your suggestion is right, I'll update this in the next release. Yours sincerely, Leoliu-oc >> Signed-off-by: leoliu-oc <leoliu-oc@zhaoxin.com> > > Assuming this goes along with a patch series that adds uses of these > definitions: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> > >> --- >> include/uapi/linux/pci_regs.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h >> index 57b8e2ffb1dd..37f3baa336d7 100644 >> --- a/include/uapi/linux/pci_regs.h >> +++ b/include/uapi/linux/pci_regs.h >> @@ -799,6 +799,11 @@ >> #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */ >> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ >> >> +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ >> +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */ >> +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */ >> +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */ > > Please squash these right up next to the other PCI_ERR_* definitions > so it's obvious that they overlap PCI_ERR_ROOT_STATUS and > PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both), > e.g., > > #define PCI_ERR_ROOT_STATUS 0x30 > #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */ > ... > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ > #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */ > #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */ > #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */ > I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2, PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of individual errors that occur on traditional PCI or PCI-x secondary bus interfaces, these registers are valid only for Bridge. Although PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they represent register definitions for different device types. Yours sincerely, Leoliu-oc >> /* Virtual Channel */ >> #define PCI_VC_PORT_CAP1 0x04 >> #define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */ >> -- >> 2.20.1 >>
On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote: > > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > > > index 57b8e2ffb1dd..37f3baa336d7 100644 > > > --- a/include/uapi/linux/pci_regs.h > > > +++ b/include/uapi/linux/pci_regs.h > > > @@ -799,6 +799,11 @@ > > > #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */ > > > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ > > > +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ > > > +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */ > > > +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */ > > > +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */ > > > > Please squash these right up next to the other PCI_ERR_* definitions > > so it's obvious that they overlap PCI_ERR_ROOT_STATUS and > > PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both), > > e.g., > > > > #define PCI_ERR_ROOT_STATUS 0x30 > > #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */ > > ... > > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ > > #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */ > > #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */ > > #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */ > > I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2, > PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of > individual errors that occur on traditional PCI or PCI-x secondary bus > interfaces, these registers are valid only for Bridge. Although > PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they > represent register definitions for different device types. Right. I just don't want the blank line in the middle because it might be mistaken for items in a different capability. All the other AER capability registers are defined together in a block, with no blank lines in the middle, so I think these new ones should be part of that block. Bjorn
在 2023/4/13 0:10, Bjorn Helgaas 写道: > On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote: > >>>> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h >>>> index 57b8e2ffb1dd..37f3baa336d7 100644 >>>> --- a/include/uapi/linux/pci_regs.h >>>> +++ b/include/uapi/linux/pci_regs.h >>>> @@ -799,6 +799,11 @@ >>>> #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */ >>>> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ >>>> +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ >>>> +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */ >>>> +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */ >>>> +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */ >>> >>> Please squash these right up next to the other PCI_ERR_* definitions >>> so it's obvious that they overlap PCI_ERR_ROOT_STATUS and >>> PCI_ERR_ROOT_ERR_SRC (which is fine since one device can't have both), >>> e.g., >>> >>> #define PCI_ERR_ROOT_STATUS 0x30 >>> #define PCI_ERR_ROOT_COR_RCV 0x00000001 /* ERR_COR Received */ >>> ... >>> #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ >>> #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */ >>> #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */ >>> #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */ >> >> I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2, >> PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of >> individual errors that occur on traditional PCI or PCI-x secondary bus >> interfaces, these registers are valid only for Bridge. Although >> PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they >> represent register definitions for different device types. > > Right. I just don't want the blank line in the middle because it > might be mistaken for items in a different capability. All the other > AER capability registers are defined together in a block, with no > blank lines in the middle, so I think these new ones should be part of > that block. > > Bjorn Ok，I see your point. Do you think this line of comment is still necessary? /* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ Yours sincerely, Leoliu-oc
On Tue, Apr 18, 2023 at 10:38:58AM +0800, LeoLiuoc wrote: > 在 2023/4/13 0:10, Bjorn Helgaas 写道: > > On Wed, Apr 12, 2023 at 05:49:55PM +0800, LeoLiuoc wrote: > > > > ... > > > > #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ > > > > #define PCI_ERR_UNCOR_MASK2 0x30 /* PCIe to PCI/PCI-X bridge */ > > > > #define PCI_ERR_UNCOR_SEVER2 0x34 /* PCIe to PCI/PCI-X bridge */ > > > > #define PCI_ERR_CAP2 0x38 /* PCIe to PCI/PCI-X bridge */ > > > > > > I don't seem to understand what you mean. PCI_ERR_UNCOR_MASK2, > > > PCI_ERR_UNCOR_SEVER2, and PCI_ERR_CAP2 represent the control and handling of > > > individual errors that occur on traditional PCI or PCI-x secondary bus > > > interfaces, these registers are valid only for Bridge. Although > > > PCI_ERR_ROOT_ERR_SRC and PCI_ERR_UNCOR_SEVER2 have the same value, they > > > represent register definitions for different device types. > > > > Right. I just don't want the blank line in the middle because it > > might be mistaken for items in a different capability. All the other > > AER capability registers are defined together in a block, with no > > blank lines in the middle, so I think these new ones should be part of > > that block. > > Ok，I see your point. Do you think this line of comment is still necessary? > /* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X > Bridge */ I suggested a trailing comment ("PCIe to PCI/PCI-X bridge"). If we use that, I don't think the other is necessary. Bjorn
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 57b8e2ffb1dd..37f3baa336d7 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -799,6 +799,11 @@ #define PCI_ERR_ROOT_AER_IRQ 0xf8000000 /* Advanced Error Interrupt Message Number */ #define PCI_ERR_ROOT_ERR_SRC 0x34 /* Error Source Identification */ +/* PCIe advanced error reporting extended capabilities for PCIe to PCI/PCI-X Bridge */ +#define PCI_ERR_UNCOR_MASK2 0x30 /* Secondary Uncorrectable Error Mask */ +#define PCI_ERR_UNCOR_SEVER2 0x34 /* Secondary Uncorrectable Error Severit */ +#define PCI_ERR_CAP2 0x38 /* Secondary Advanced Error Capabilities */ + /* Virtual Channel */ #define PCI_VC_PORT_CAP1 0x04 #define PCI_VC_CAP1_EVCC 0x00000007 /* extended VC count */