| Message ID | 20180821104451.25769-1-vaibhav@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phb4: Enable PHB MMIO-0/1 Bars only when mmio window exists | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/make_check | success | Test make_check on branch master |
On Tue, Aug 21, 2018 at 8:44 PM, Vaibhav Jain <vaibhav@linux.ibm.com> wrote: > Presently phb4_probe_stack() will always enable PHB MMIO0/1 windows > even if they doesn't exist in phy_map. Hence we do some minor shuffling > in the phb4_probe_stack() so that MMIO-0/1 Bars are only enabled if > there corresponding MMIO window exists in the phy_map. In what circumstances do we have PHBs that aren't in the phys map? > Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> > --- > hw/phb4.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hw/phb4.c b/hw/phb4.c > index d1245dce..935b0e92 100644 > --- a/hw/phb4.c > +++ b/hw/phb4.c > @@ -5580,25 +5580,28 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index, > /* Same with MMIO windows */ > phys_map_get(gcid, PHB4_64BIT_MMIO, phb_num, &mmio0_bar, &mmio0_sz); > mmio0_bmask = (~(mmio0_sz - 1)) & 0x00FFFFFFFFFFFFFFULL; > - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0, mmio0_bar << 8); > - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0_MASK, mmio0_bmask << 8); > > phys_map_get(gcid, PHB4_32BIT_MMIO, phb_num, &mmio1_bar, &mmio1_sz); > mmio1_bmask = (~(mmio1_sz - 1)) & 0x00FFFFFFFFFFFFFFULL; > - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1, mmio1_bar << 8); > - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1_MASK, mmio1_bmask << 8); > - bar_en |= XPEC_NEST_STK_BAR_EN_MMIO0 | XPEC_NEST_STK_BAR_EN_MMIO1; I think you can remove just this line and drop the rest of the changes in the patch. I'd rather we kept the XSCOM writes here so that we know the BAR and MASK registers are explicitly set to zero even when the BAR is disabled. > /* Build MMIO windows list */ > mmio_win_sz = 0; > if (mmio0_bar) { > mmio_win[mmio_win_sz++] = mmio0_bar; > mmio_win[mmio_win_sz++] = mmio0_sz; > + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0, > + mmio0_bar << 8); > + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0_MASK, > + mmio0_bmask << 8); > bar_en |= XPEC_NEST_STK_BAR_EN_MMIO0; > } > if (mmio1_bar) { > mmio_win[mmio_win_sz++] = mmio1_bar; > mmio_win[mmio_win_sz++] = mmio1_sz; > + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1, > + mmio1_bar << 8); > + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1_MASK, > + mmio1_bmask << 8); > bar_en |= XPEC_NEST_STK_BAR_EN_MMIO1; > } > > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Thanks for reviewing the patch Oliver Oliver <oohall@gmail.com> writes: > In what circumstances do we have PHBs that aren't in the phys map? AFAIK None right now. But still theres certainly a possibility of having slots that only supports cards with one of two 64/32 bit mmio windows. > I think you can remove just this line and drop the rest of the changes > in the patch. I'd rather we kept the XSCOM writes here so that we know > the BAR and MASK registers are explicitly set to zero even when the > BAR is disabled. Thanks for the suggestion. I will send a v2 incorporating this change.
diff --git a/hw/phb4.c b/hw/phb4.c index d1245dce..935b0e92 100644 --- a/hw/phb4.c +++ b/hw/phb4.c @@ -5580,25 +5580,28 @@ static void phb4_probe_stack(struct dt_node *stk_node, uint32_t pec_index, /* Same with MMIO windows */ phys_map_get(gcid, PHB4_64BIT_MMIO, phb_num, &mmio0_bar, &mmio0_sz); mmio0_bmask = (~(mmio0_sz - 1)) & 0x00FFFFFFFFFFFFFFULL; - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0, mmio0_bar << 8); - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0_MASK, mmio0_bmask << 8); phys_map_get(gcid, PHB4_32BIT_MMIO, phb_num, &mmio1_bar, &mmio1_sz); mmio1_bmask = (~(mmio1_sz - 1)) & 0x00FFFFFFFFFFFFFFULL; - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1, mmio1_bar << 8); - xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1_MASK, mmio1_bmask << 8); - bar_en |= XPEC_NEST_STK_BAR_EN_MMIO0 | XPEC_NEST_STK_BAR_EN_MMIO1; /* Build MMIO windows list */ mmio_win_sz = 0; if (mmio0_bar) { mmio_win[mmio_win_sz++] = mmio0_bar; mmio_win[mmio_win_sz++] = mmio0_sz; + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0, + mmio0_bar << 8); + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR0_MASK, + mmio0_bmask << 8); bar_en |= XPEC_NEST_STK_BAR_EN_MMIO0; } if (mmio1_bar) { mmio_win[mmio_win_sz++] = mmio1_bar; mmio_win[mmio_win_sz++] = mmio1_sz; + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1, + mmio1_bar << 8); + xscom_write(gcid, nest_stack + XPEC_NEST_STK_MMIO_BAR1_MASK, + mmio1_bmask << 8); bar_en |= XPEC_NEST_STK_BAR_EN_MMIO1; }
Presently phb4_probe_stack() will always enable PHB MMIO0/1 windows even if they doesn't exist in phy_map. Hence we do some minor shuffling in the phb4_probe_stack() so that MMIO-0/1 Bars are only enabled if there corresponding MMIO window exists in the phy_map. Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com> --- hw/phb4.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)