[Skiboot] [RFC PATCH] Remove POWER9 DD1 support

[Cédric Le Goater]

On 1/15/19 8:07 AM, Nicholas Piggin wrote:
> Cédric Le Goater's on January 9, 2019 3:28 am:
>> Hello Nicholas,
>>
>> On 1/7/19 3:09 PM, Nicholas Piggin wrote:
>>> There's a couple of cases I'm not sure if they're still needed (marked
>>> with XXX).
>>>
>>> Hostboot and Linux have removed DD1 support so there's not much point
>>> keeping it around.
>>
>> Some comments on XIVE below. 
> 
> Thanks!
> 
>>> diff --git a/hw/xive.c b/hw/xive.c
>>> index 515f154d7..810538725 100644
>>> --- a/hw/xive.c
>>> +++ b/hw/xive.c
>>> @@ -487,8 +487,7 @@ struct xive {
>>>  	void		*q_ovf;
>>>  };
>>>  
>>> -#define XIVE_CAN_STORE_EOI(x) \
>>> -	(XIVE_STORE_EOI_ENABLED && ((x)->rev >= XIVE_REV_2))
>>> +#define XIVE_CAN_STORE_EOI(x) XIVE_STORE_EOI_ENABLED
>>
>> OK. Let's keep the macro. P10 should have Store EOI.
> 
> Sounds good.
> 
>>>  /* Global DT node */
>>>  static struct dt_node *xive_dt_node;
>>> @@ -1521,7 +1520,7 @@ static bool xive_set_vsd(struct xive *x, uint32_t tbl, uint32_t idx, uint64_t v)
>>>  		  SETFIELD(VST_TABLE_OFFSET, 0ull, idx));
>>>  	if (x->last_reg_error)
>>>  		return false;
>>> -	/* Hack to workaround DD1 issue with NVT in VC in DD1 */
>>> +	/* Hack to workaround DD1 issue with NVT in VC in DD1 XXX still needed? */
>>>  	if (tbl == VST_TSEL_VPDT)
>>>  		xive_regw(x, VC_VSD_TABLE_DATA, v | VSD_TSIZE);
>>
>> Yes. I will check. It has been a pain to model in QEMU.
> 
> Yes, you mean it is still needed? Comment can just be updated to remove
> DD1 reference.

the comment and the code :

	/* Hack to workaround DD1 issue with NVT in VC in DD1 */
 	if (tbl == VST_TSEL_VPDT)
  		xive_regw(x, VC_VSD_TABLE_DATA, v | VSD_TSIZE);

can be removed. 

What it does is to set the NVT table size to all ones. Which is the 
maximum size (2^43) if I am correct.

This is clearly a HW workaround.

Thanks,

C.

> 
>>>  	else
>>> @@ -1743,10 +1742,6 @@ static bool xive_config_init(struct xive *x)
>>
>>
>> There is a check on XIVE_REV_2 that we could clean up :
>>
>> 	if (x->rev >= XIVE_REV_2) {
>> 		val = SETFIELD(PC_TCTXT_INIT_AGE, val, 0x2);
>> 		val |= PC_TCTXT_CFG_LGS_EN;
>> 		/* Disable pressure relief as we hijack the field in the VPs */
>> 		val &= ~PC_TCTXT_CFG_STORE_ACK;
>> 	}
>>
>> I think.
> 
> I will do that.
> 
>>> @@ -2840,10 +2834,8 @@ static struct xive *init_one_xive(struct dt_node *np)
>>>  
>>>  	x->rev = XIVE_REV_UNKNOWN;
>>>  	if (chip->type == PROC_CHIP_P9_NIMBUS) {
>>> -		if ((chip->ec_level & 0xf0) == 0x10)
>>> -			x->rev = XIVE_REV_1;
>>> -		else if ((chip->ec_level & 0xf0) == 0x20)
>>> -			x->rev = XIVE_REV_2;
>>> +		assert((chip->ec_level & 0xf0) != 0x10);
>>> +		x->rev = XIVE_REV_2;
>>>  	} else if (chip->type == PROC_CHIP_P9_CUMULUS)
>>>  		x->rev = XIVE_REV_2;
>>
>> So all P9s are XIVE_REV_2 now ? 
> 
> Yes with dropping P9N DD1 it seems so.
> 
> Thanks,
> Nick
>