Re: puc(4): 64-bit BARs
That's quite a hack you wrote, but I think any nicer way of doing it is
going to require a big rewrite..
Patrick Wildt wrote:
> as it turns out, puc(4) has trouble with 64-bit BARs. The issue is that
> puc(4) tries to map every BAR before doing anything else. While iter-
> ating over the BARs it assumes the next BAR is always 4 bytes after the
> other. For 64-bit BARs that's wrong. On a device with a 64-bit BAR, it
> will map the BAR correctly, and then using the 0x4 offset try to map an-
> other BAR, thus breaking the previously correctly mapped bar.
>
> Now I'm not sure this is the best fix, but I think the easiest way is to
> just skip that particular entry. Thus, with two 64-bit BARs, sc->sc_bar
> mappings[1] and [3] will be empty. That's fine since the pucdata.c
> entries reference the absolute BAR address, like 0x10 and 0x18, and the
> mapping to the index of the array is done using that absolute value:
>
> #define PUC_PORT_BAR_INDEX(bar) (((bar) - PCI_MAPREG_START) / 4)
>
> bar = PUC_PORT_BAR_INDEX(desc->ports[i].bar);
> if (!sc->sc_bar_mappings[bar].mapped) {
> ...
>
> Feedback?
>
> Patrick
>
> diff --git a/sys/dev/pci/puc.c b/sys/dev/pci/puc.c
> index 2c5f3065177..7ec25ecaad2 100644
> --- a/sys/dev/pci/puc.c
> +++ b/sys/dev/pci/puc.c
> @@ -163,8 +163,11 @@ puc_pci_attach(struct device *parent, struct device
> *self, void *aux)
> 0, >sc_bar_mappings[i].t, >sc_bar_mappings[i].h,
> >sc_bar_mappings[i].a, >sc_bar_mappings[i].s, 0)
> == 0);
> - if (sc->sc_bar_mappings[i].mapped)
> + if (sc->sc_bar_mappings[i].mapped) {
> + if (type == PCI_MAPREG_MEM_TYPE_64BIT)
> + i++;
> continue;
> + }
>
> #if NCOM > 0
> /*
> @@ -184,6 +187,8 @@ puc_pci_attach(struct device *parent, struct device
> *self, void *aux)
> sc->sc_bar_mappings[i].h = comconsioh;
> sc->sc_bar_mappings[i].s = COM_NPORTS;
> sc->sc_bar_mappings[i].mapped = 1;
> + if (type == PCI_MAPREG_MEM_TYPE_64BIT)
> + i++;
> continue;
> }
> #endif
>
Re: puc(4): 64-bit BARs
> Date: Mon, 2 Mar 2020 13:56:11 +0100
> From: Patrick Wildt
>
> Hi,
>
> as it turns out, puc(4) has trouble with 64-bit BARs. The issue is that
> puc(4) tries to map every BAR before doing anything else. While iter-
> ating over the BARs it assumes the next BAR is always 4 bytes after the
> other. For 64-bit BARs that's wrong. On a device with a 64-bit BAR, it
> will map the BAR correctly, and then using the 0x4 offset try to map an-
> other BAR, thus breaking the previously correctly mapped bar.
>
> Now I'm not sure this is the best fix, but I think the easiest way is to
> just skip that particular entry. Thus, with two 64-bit BARs, sc->sc_bar
> mappings[1] and [3] will be empty. That's fine since the pucdata.c
> entries reference the absolute BAR address, like 0x10 and 0x18, and the
> mapping to the index of the array is done using that absolute value:
>
> #define PUC_PORT_BAR_INDEX(bar) (((bar) - PCI_MAPREG_START) / 4)
>
> bar = PUC_PORT_BAR_INDEX(desc->ports[i].bar);
> if (!sc->sc_bar_mappings[bar].mapped) {
> ...
>
> Feedback?
ok kettenis@
> Patrick
>
> diff --git a/sys/dev/pci/puc.c b/sys/dev/pci/puc.c
> index 2c5f3065177..7ec25ecaad2 100644
> --- a/sys/dev/pci/puc.c
> +++ b/sys/dev/pci/puc.c
> @@ -163,8 +163,11 @@ puc_pci_attach(struct device *parent, struct device
> *self, void *aux)
> 0, >sc_bar_mappings[i].t, >sc_bar_mappings[i].h,
> >sc_bar_mappings[i].a, >sc_bar_mappings[i].s, 0)
> == 0);
> - if (sc->sc_bar_mappings[i].mapped)
> + if (sc->sc_bar_mappings[i].mapped) {
> + if (type == PCI_MAPREG_MEM_TYPE_64BIT)
> + i++;
> continue;
> + }
>
> #if NCOM > 0
> /*
> @@ -184,6 +187,8 @@ puc_pci_attach(struct device *parent, struct device
> *self, void *aux)
> sc->sc_bar_mappings[i].h = comconsioh;
> sc->sc_bar_mappings[i].s = COM_NPORTS;
> sc->sc_bar_mappings[i].mapped = 1;
> + if (type == PCI_MAPREG_MEM_TYPE_64BIT)
> + i++;
> continue;
> }
> #endif
>
>
puc(4): 64-bit BARs
Hi,
as it turns out, puc(4) has trouble with 64-bit BARs. The issue is that
puc(4) tries to map every BAR before doing anything else. While iter-
ating over the BARs it assumes the next BAR is always 4 bytes after the
other. For 64-bit BARs that's wrong. On a device with a 64-bit BAR, it
will map the BAR correctly, and then using the 0x4 offset try to map an-
other BAR, thus breaking the previously correctly mapped bar.
Now I'm not sure this is the best fix, but I think the easiest way is to
just skip that particular entry. Thus, with two 64-bit BARs, sc->sc_bar
mappings[1] and [3] will be empty. That's fine since the pucdata.c
entries reference the absolute BAR address, like 0x10 and 0x18, and the
mapping to the index of the array is done using that absolute value:
#define PUC_PORT_BAR_INDEX(bar) (((bar) - PCI_MAPREG_START) / 4)
bar = PUC_PORT_BAR_INDEX(desc->ports[i].bar);
if (!sc->sc_bar_mappings[bar].mapped) {
...
Feedback?
Patrick
diff --git a/sys/dev/pci/puc.c b/sys/dev/pci/puc.c
index 2c5f3065177..7ec25ecaad2 100644
--- a/sys/dev/pci/puc.c
+++ b/sys/dev/pci/puc.c
@@ -163,8 +163,11 @@ puc_pci_attach(struct device *parent, struct device *self,
void *aux)
0, >sc_bar_mappings[i].t, >sc_bar_mappings[i].h,
>sc_bar_mappings[i].a, >sc_bar_mappings[i].s, 0)
== 0);
- if (sc->sc_bar_mappings[i].mapped)
+ if (sc->sc_bar_mappings[i].mapped) {
+ if (type == PCI_MAPREG_MEM_TYPE_64BIT)
+ i++;
continue;
+ }
#if NCOM > 0
/*
@@ -184,6 +187,8 @@ puc_pci_attach(struct device *parent, struct device *self,
void *aux)
sc->sc_bar_mappings[i].h = comconsioh;
sc->sc_bar_mappings[i].s = COM_NPORTS;
sc->sc_bar_mappings[i].mapped = 1;
+ if (type == PCI_MAPREG_MEM_TYPE_64BIT)
+ i++;
continue;
}
#endif
