[Skiboot] [RFC PATCH] Remove POWER9 DD1 support

[Nicholas Piggin]

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.

>>  	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