* [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation
@ 2018-11-09 7:58 Ard Biesheuvel
2018-11-09 13:34 ` Leif Lindholm
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2018-11-09 7:58 UTC (permalink / raw)
To: edk2-devel
Commit 9dd8190e4995 ("Silicon/SynQuacer: tweak PCI I/O windows for
ACPI/Linux support") updated the min/max/offset definitions for the
PCIe I/O resource windows on SynQuacer, and updated the read path of
the platform's EfiCpuIo2 protocol implementation, but failed to update
the write path as well, resulting in spurious errors if when attempting
to write to PCIe I/O ports on PCIe RC #1, which uses translation for the
I/O BAR window.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
v2: use helper function and temp vars
Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c | 62 ++++++++++++--------
1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
index 736b20cd5129..049657231cab 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
@@ -354,6 +354,37 @@ CpuMemoryServiceWrite (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+TranslateIoAddress (
+ IN OUT UINT64 *Address
+ )
+{
+ UINT64 Start;
+ UINT64 End;
+ UINT64 Shift;
+
+ Start = SYNQUACER_PCI_SEG0_PORTIO_MIN + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
+ End = SYNQUACER_PCI_SEG0_PORTIO_MAX + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
+ Shift = SYNQUACER_PCI_SEG0_PORTIO_MEMBASE - SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
+
+ if (*Address >= Start && *Address <= End) {
+ *Address += Shift;
+ return EFI_SUCCESS;
+ }
+
+ Start = SYNQUACER_PCI_SEG1_PORTIO_MIN + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
+ End = SYNQUACER_PCI_SEG1_PORTIO_MAX + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
+ Shift = SYNQUACER_PCI_SEG1_PORTIO_MEMBASE - SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
+
+ if (*Address >= Start && *Address <= End) {
+ *Address += Shift;
+ return EFI_SUCCESS;
+ }
+ ASSERT (FALSE);
+ return EFI_INVALID_PARAMETER;
+}
+
/**
Reads I/O registers.
@@ -415,22 +445,9 @@ CpuIoServiceRead (
return Status;
}
- if ((Address >= (SYNQUACER_PCI_SEG0_PORTIO_MIN +
- SYNQUACER_PCI_SEG0_PORTIO_OFFSET)) &&
- (Address <= (SYNQUACER_PCI_SEG0_PORTIO_MAX +
- SYNQUACER_PCI_SEG0_PORTIO_OFFSET))) {
- Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE -
- SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
- } else if ((Address >= (SYNQUACER_PCI_SEG1_PORTIO_MIN +
- SYNQUACER_PCI_SEG1_PORTIO_OFFSET)) &&
- (Address <= (SYNQUACER_PCI_SEG1_PORTIO_MAX +
- SYNQUACER_PCI_SEG1_PORTIO_OFFSET))) {
- Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE -
- SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
-
- } else {
- ASSERT (FALSE);
- return EFI_INVALID_PARAMETER;
+ Status = TranslateIoAddress (&Address);
+ if (EFI_ERROR (Status)) {
+ return Status;
}
//
@@ -518,16 +535,9 @@ CpuIoServiceWrite (
return Status;
}
- if ((Address >= SYNQUACER_PCI_SEG0_PORTIO_MIN) &&
- (Address <= SYNQUACER_PCI_SEG0_PORTIO_MAX)) {
- Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE;
- } else if ((Address >= SYNQUACER_PCI_SEG1_PORTIO_MIN) &&
- (Address <= SYNQUACER_PCI_SEG1_PORTIO_MAX)) {
- Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE;
-
- } else {
- ASSERT (FALSE);
- return EFI_INVALID_PARAMETER;
+ Status = TranslateIoAddress (&Address);
+ if (EFI_ERROR (Status)) {
+ return Status;
}
//
--
2.19.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation
2018-11-09 7:58 [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation Ard Biesheuvel
@ 2018-11-09 13:34 ` Leif Lindholm
2018-11-19 18:22 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Leif Lindholm @ 2018-11-09 13:34 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel
On Fri, Nov 09, 2018 at 08:58:48AM +0100, Ard Biesheuvel wrote:
> Commit 9dd8190e4995 ("Silicon/SynQuacer: tweak PCI I/O windows for
> ACPI/Linux support") updated the min/max/offset definitions for the
> PCIe I/O resource windows on SynQuacer, and updated the read path of
> the platform's EfiCpuIo2 protocol implementation, but failed to update
> the write path as well, resulting in spurious errors if when attempting
> to write to PCIe I/O ports on PCIe RC #1, which uses translation for the
> I/O BAR window.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Much neater, thank you.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> v2: use helper function and temp vars
>
> Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c | 62 ++++++++++++--------
> 1 file changed, 37 insertions(+), 26 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> index 736b20cd5129..049657231cab 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> @@ -354,6 +354,37 @@ CpuMemoryServiceWrite (
> return EFI_SUCCESS;
> }
>
> +STATIC
> +EFI_STATUS
> +TranslateIoAddress (
> + IN OUT UINT64 *Address
> + )
> +{
> + UINT64 Start;
> + UINT64 End;
> + UINT64 Shift;
> +
> + Start = SYNQUACER_PCI_SEG0_PORTIO_MIN + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> + End = SYNQUACER_PCI_SEG0_PORTIO_MAX + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> + Shift = SYNQUACER_PCI_SEG0_PORTIO_MEMBASE - SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> +
> + if (*Address >= Start && *Address <= End) {
> + *Address += Shift;
> + return EFI_SUCCESS;
> + }
> +
> + Start = SYNQUACER_PCI_SEG1_PORTIO_MIN + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> + End = SYNQUACER_PCI_SEG1_PORTIO_MAX + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> + Shift = SYNQUACER_PCI_SEG1_PORTIO_MEMBASE - SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> +
> + if (*Address >= Start && *Address <= End) {
> + *Address += Shift;
> + return EFI_SUCCESS;
> + }
> + ASSERT (FALSE);
> + return EFI_INVALID_PARAMETER;
> +}
> +
> /**
> Reads I/O registers.
>
> @@ -415,22 +445,9 @@ CpuIoServiceRead (
> return Status;
> }
>
> - if ((Address >= (SYNQUACER_PCI_SEG0_PORTIO_MIN +
> - SYNQUACER_PCI_SEG0_PORTIO_OFFSET)) &&
> - (Address <= (SYNQUACER_PCI_SEG0_PORTIO_MAX +
> - SYNQUACER_PCI_SEG0_PORTIO_OFFSET))) {
> - Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE -
> - SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> - } else if ((Address >= (SYNQUACER_PCI_SEG1_PORTIO_MIN +
> - SYNQUACER_PCI_SEG1_PORTIO_OFFSET)) &&
> - (Address <= (SYNQUACER_PCI_SEG1_PORTIO_MAX +
> - SYNQUACER_PCI_SEG1_PORTIO_OFFSET))) {
> - Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE -
> - SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> -
> - } else {
> - ASSERT (FALSE);
> - return EFI_INVALID_PARAMETER;
> + Status = TranslateIoAddress (&Address);
> + if (EFI_ERROR (Status)) {
> + return Status;
> }
>
> //
> @@ -518,16 +535,9 @@ CpuIoServiceWrite (
> return Status;
> }
>
> - if ((Address >= SYNQUACER_PCI_SEG0_PORTIO_MIN) &&
> - (Address <= SYNQUACER_PCI_SEG0_PORTIO_MAX)) {
> - Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE;
> - } else if ((Address >= SYNQUACER_PCI_SEG1_PORTIO_MIN) &&
> - (Address <= SYNQUACER_PCI_SEG1_PORTIO_MAX)) {
> - Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE;
> -
> - } else {
> - ASSERT (FALSE);
> - return EFI_INVALID_PARAMETER;
> + Status = TranslateIoAddress (&Address);
> + if (EFI_ERROR (Status)) {
> + return Status;
> }
>
> //
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation
2018-11-09 13:34 ` Leif Lindholm
@ 2018-11-19 18:22 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2018-11-19 18:22 UTC (permalink / raw)
To: Leif Lindholm; +Cc: edk2-devel@lists.01.org
On Fri, 9 Nov 2018 at 05:34, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
>
> On Fri, Nov 09, 2018 at 08:58:48AM +0100, Ard Biesheuvel wrote:
> > Commit 9dd8190e4995 ("Silicon/SynQuacer: tweak PCI I/O windows for
> > ACPI/Linux support") updated the min/max/offset definitions for the
> > PCIe I/O resource windows on SynQuacer, and updated the read path of
> > the platform's EfiCpuIo2 protocol implementation, but failed to update
> > the write path as well, resulting in spurious errors if when attempting
> > to write to PCIe I/O ports on PCIe RC #1, which uses translation for the
> > I/O BAR window.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Much neater, thank you.
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
Thanks.
Pushed as 91b0299223a619cfec05a638f2c4197266d3cf1e
> > ---
> > v2: use helper function and temp vars
> >
> > Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c | 62 ++++++++++++--------
> > 1 file changed, 37 insertions(+), 26 deletions(-)
> >
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> > index 736b20cd5129..049657231cab 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/SynQuacerPciCpuIo2Dxe/SynQuacerPciCpuIo2Dxe.c
> > @@ -354,6 +354,37 @@ CpuMemoryServiceWrite (
> > return EFI_SUCCESS;
> > }
> >
> > +STATIC
> > +EFI_STATUS
> > +TranslateIoAddress (
> > + IN OUT UINT64 *Address
> > + )
> > +{
> > + UINT64 Start;
> > + UINT64 End;
> > + UINT64 Shift;
> > +
> > + Start = SYNQUACER_PCI_SEG0_PORTIO_MIN + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> > + End = SYNQUACER_PCI_SEG0_PORTIO_MAX + SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> > + Shift = SYNQUACER_PCI_SEG0_PORTIO_MEMBASE - SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> > +
> > + if (*Address >= Start && *Address <= End) {
> > + *Address += Shift;
> > + return EFI_SUCCESS;
> > + }
> > +
> > + Start = SYNQUACER_PCI_SEG1_PORTIO_MIN + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> > + End = SYNQUACER_PCI_SEG1_PORTIO_MAX + SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> > + Shift = SYNQUACER_PCI_SEG1_PORTIO_MEMBASE - SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> > +
> > + if (*Address >= Start && *Address <= End) {
> > + *Address += Shift;
> > + return EFI_SUCCESS;
> > + }
> > + ASSERT (FALSE);
> > + return EFI_INVALID_PARAMETER;
> > +}
> > +
> > /**
> > Reads I/O registers.
> >
> > @@ -415,22 +445,9 @@ CpuIoServiceRead (
> > return Status;
> > }
> >
> > - if ((Address >= (SYNQUACER_PCI_SEG0_PORTIO_MIN +
> > - SYNQUACER_PCI_SEG0_PORTIO_OFFSET)) &&
> > - (Address <= (SYNQUACER_PCI_SEG0_PORTIO_MAX +
> > - SYNQUACER_PCI_SEG0_PORTIO_OFFSET))) {
> > - Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE -
> > - SYNQUACER_PCI_SEG0_PORTIO_OFFSET;
> > - } else if ((Address >= (SYNQUACER_PCI_SEG1_PORTIO_MIN +
> > - SYNQUACER_PCI_SEG1_PORTIO_OFFSET)) &&
> > - (Address <= (SYNQUACER_PCI_SEG1_PORTIO_MAX +
> > - SYNQUACER_PCI_SEG1_PORTIO_OFFSET))) {
> > - Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE -
> > - SYNQUACER_PCI_SEG1_PORTIO_OFFSET;
> > -
> > - } else {
> > - ASSERT (FALSE);
> > - return EFI_INVALID_PARAMETER;
> > + Status = TranslateIoAddress (&Address);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > }
> >
> > //
> > @@ -518,16 +535,9 @@ CpuIoServiceWrite (
> > return Status;
> > }
> >
> > - if ((Address >= SYNQUACER_PCI_SEG0_PORTIO_MIN) &&
> > - (Address <= SYNQUACER_PCI_SEG0_PORTIO_MAX)) {
> > - Address += SYNQUACER_PCI_SEG0_PORTIO_MEMBASE;
> > - } else if ((Address >= SYNQUACER_PCI_SEG1_PORTIO_MIN) &&
> > - (Address <= SYNQUACER_PCI_SEG1_PORTIO_MAX)) {
> > - Address += SYNQUACER_PCI_SEG1_PORTIO_MEMBASE;
> > -
> > - } else {
> > - ASSERT (FALSE);
> > - return EFI_INVALID_PARAMETER;
> > + Status = TranslateIoAddress (&Address);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > }
> >
> > //
> > --
> > 2.19.1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-19 18:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-09 7:58 [PATCH v2 edk2-platforms] Silicon/SynQuacerPciCpuIo2Dxe: fix PCIe I/O translation Ard Biesheuvel
2018-11-09 13:34 ` Leif Lindholm
2018-11-19 18:22 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox