Thanks for the patches !
On 26/06/17 04:50AM, Saif Abrar wrote:
> diff --git a/hw/pci-host/pnv_phb.c b/hw/pci-host/pnv_phb.c
> index 0b556d1bf5..d4f452d7b2 100644
> --- a/hw/pci-host/pnv_phb.c
> +++ b/hw/pci-host/pnv_phb.c
> @@ -232,6 +232,7 @@ static void pnv_phb_root_port_reset_hold(Object *obj,
> ResetType type)
> pci_set_long(conf + PCI_PREF_BASE_UPPER32, 0x1); /* Hack */
> pci_set_long(conf + PCI_PREF_LIMIT_UPPER32, 0xffffffff);
> pci_config_set_interrupt_pin(conf, 0);
> + pnv_phb4_cfg_core_reset(d);
nit: phb4 reset in common phb path ? will it affect phb3 too ?
> }
>
> <...snip...>
> +void pnv_phb4_cfg_core_reset(PCIDevice *d)
> +{
> + uint8_t *conf = d->config;
> + uint32_t exp_offset = get_exp_offset(d);
> +
> + pci_set_word(conf + PCI_COMMAND, PCI_COMMAND_SERR);
> + pci_set_word(conf + PCI_STATUS, PCI_STATUS_CAP_LIST);
> + pci_set_long(conf + PCI_CLASS_REVISION, 0x06040000);
> + pci_set_long(conf + PCI_CACHE_LINE_SIZE, BIT(16));
> + pci_set_word(conf + PCI_MEMORY_BASE, BIT(4));
> + pci_set_word(conf + PCI_PREF_MEMORY_BASE, BIT(0) | BIT(4));
> + pci_set_word(conf + PCI_PREF_MEMORY_LIMIT, PCI_PREF_RANGE_TYPE_64);
> + pci_set_long(conf + PCI_CAPABILITY_LIST, BIT(6));
> + pci_set_word(conf + PCI_BRIDGE_CONTROL, PCI_BRIDGE_CTL_SERR);
> + pci_set_long(conf + PCI_BRIDGE_CONTROL + PCI_PM_PMC, 0xC8034801);
> +
> + pci_set_long(conf + exp_offset, 0x420010);
> + pci_set_long(conf + exp_offset + PCI_EXP_DEVCAP, 0x8022);
> + pci_set_long(conf + exp_offset + PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_EXT_TAG
> + | PCI_EXP_DEVCTL_PAYLOAD_512B);
> + pci_set_long(conf + exp_offset + PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_LBNC
> + | PCI_EXP_LNKCAP_DLLLARC | BIT(8) |
> PCI_EXP_LNKCAP_SLS_32_0GB);
> + pci_set_word(conf + exp_offset + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_RCB);
> + pci_set_long(conf + exp_offset + PCI_EXP_SLTCTL,
> +
> PCI_EXP_SLTCTL_ASPL_DISABLE);
> + pci_set_long(conf + exp_offset + PCI_EXP_DEVCAP2, BIT(16)
> + | PCI_EXP_DEVCAP2_ARI | PCI_EXP_DEVCAP2_COMP_TMOUT_DIS |
> 0xF);
> + pci_set_long(conf + exp_offset + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ARI);
> + pci_set_long(conf + exp_offset + PCI_EXP_LNKCAP2, BIT(23)
> + | PCI_EXP_LNKCAP2_SLS_32_0GB
> + | PCI_EXP_LNKCAP2_SLS_16_0GB |
> PCI_EXP_LNKCAP2_SLS_8_0GB
> + | PCI_EXP_LNKCAP2_SLS_5_0GB |
> PCI_EXP_LNKCAP2_SLS_2_5GB);
> + pci_set_long(conf + PHB_AER_ECAP, PCI_EXT_CAP(0x1, 0x1, 0x148));
> + pci_set_long(conf + PHB_SEC_ECAP, (0x1A0 << 20) | BIT(16)
> + |
> PCI_EXT_CAP_ID_SECPCI);
> + pci_set_long(conf + PHB_LMR_ECAP, 0x1E810027);
> + /* LMR - Margining Lane Control / Status Register # 2 to 16 */
> + for (int i = PHB_LMR_CTLSTA_2 ; i <= PHB_LMR_CTLSTA_16 ; i += 4) {
> + pci_set_long(conf + i, 0x9C38);
> + }
> +
> + pci_set_long(conf + PHB_DLF_ECAP, 0x1F410025);
> + pci_set_long(conf + PHB_DLF_CAP, 0x80000001);
> + pci_set_long(conf + P16_ECAP, 0x22410026);
> + pci_set_long(conf + P32_ECAP, 0x1002A);
> + pci_set_long(conf + P32_CAP, 0x103);
> +}
> +
> +static void pnv_phb4_pbl_core_reset(PnvPHB4 *phb)
> +{
> + /* Zero all registers initially */
> + for (int i = PHB_PBL_CONTROL ; i <= PHB_PBL_ERR1_STATUS_MASK ; i += 8) {
nit: should we use PNV_PHB4_NUM_REGS here, since we want to set all
phb->regs to 0 here ?
#define PNV_PHB4_NUM_REGS (0x3000 >> 3)
> + phb->regs[i >> 3] = 0x0;
> + }
> +
> + /* Set specific register values */
> + phb->regs[PHB_PBL_CONTROL >> 3] = 0xC009000000000000;
This reset value is different for PHB4 and PHB5.
The PHB4 spec that i am referring to marks the bits 8:15 as reserved,
and unimplemented. While PHB5 suggests the reset value of 0x9.
I am okay with considering those bits as implementation defined and let
qemu set those bits, but maybe it's better to separate it out in
pnv_phb5.c ?
> + phb->regs[PHB_PBL_TIMEOUT_CTRL >> 3] = 0x2020000000000000;
> + phb->regs[PHB_PBL_NPTAG_ENABLE >> 3] = 0xFFFFFFFF00000000;
Same 8:31 are reserved bits in PHB4, while not in PHB5.
Maybe separating out PHB5 from PHB4 helps in such cases.
> + phb->regs[PHB_PBL_SYS_LINK_INIT >> 3] = 0x80088B4642473000;
> +}
> +
> <...snip...>
> +static void phb5_tests(void)
> +{
> + QTestState *qts = NULL;
> +
> + qts = qtest_initf("-machine powernv10 -accel tcg");
> +
> + /* Check reset value of a register */
> + phb5_reset_test(qts);
> +
> + qtest_quit(qts);
> +}
> +
> /* Assert that 'PHB - Version Register' bits[24:31] are as expected */
> static void phb_version_test(const void *data)
> {
> @@ -59,7 +83,7 @@ static void phb_version_test(const void *data)
> expected_ver = 0xA4;
> } else if (chip->chip_type == PNV_CHIP_POWER10) {
> machine = "powernv10";
This patch is giving an error on compilation now in upstream, due to
introduction of 'pnv_get_machine_type'. We can use
'pnv_get_machine_type' here in the start in place of machine = "powernv8".
What do you say ?
Thanks,
- Aditya G
