Re: [PATCH v5 10/31] opal: add MPIPL interface definitions

[Hari Bathini <hbathini@linux.ibm.com>]

On 03/09/19 4:40 PM, Michael Ellerman wrote:
> Hari Bathini <hbathini@linux.ibm.com> writes:
>> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
>> index 383242e..c8a5665 100644
>> --- a/arch/powerpc/include/asm/opal-api.h
>> +++ b/arch/powerpc/include/asm/opal-api.h
>> @@ -980,6 +983,50 @@ struct opal_sg_list {
>>  };
>>  
>>  /*
>> + * Firmware-Assisted Dump (FADump) using MPIPL
>> + */
>> +
>> +/* MPIPL update operations */
>> +enum opal_mpipl_ops {
>> +	OPAL_MPIPL_ADD_RANGE			= 0,
>> +	OPAL_MPIPL_REMOVE_RANGE			= 1,
>> +	OPAL_MPIPL_REMOVE_ALL			= 2,
>> +	OPAL_MPIPL_FREE_PRESERVED_MEMORY	= 3,
>> +};
>> +
>> +/*
>> + * Each tag maps to a metadata type. Use these tags to register/query
>> + * corresponding metadata address with/from OPAL.
>> + */
>> +enum opal_mpipl_tags {
>> +	OPAL_MPIPL_TAG_CPU		= 0,
>> +	OPAL_MPIPL_TAG_OPAL		= 1,
>> +	OPAL_MPIPL_TAG_KERNEL		= 2,
>> +	OPAL_MPIPL_TAG_BOOT_MEM		= 3,
>> +};
>> +
>> +/* Preserved memory details */
>> +struct opal_mpipl_region {
>> +	__be64	src;
>> +	__be64	dest;
>> +	__be64	size;
>> +};
>> +
>> +/* FADump structure format version */
>> +#define MPIPL_FADUMP_VERSION			0x01
>> +
>> +/* Metadata provided by OPAL. */
>> +struct opal_mpipl_fadump {
>> +	u8				version;
>> +	u8				reserved[7];
>> +	__be32				crashing_pir;
>> +	__be32				cpu_data_version;
>> +	__be32				cpu_data_size;
>> +	__be32				region_cnt;
>> +	struct opal_mpipl_region	region[];
>> +} __attribute__((packed));
>> +
> 
> The above hunk is in the wrong place vs the skiboot header. Please put
> things in exactly the same place in the skiboot and kernel versions of
> the header.
> 
> After your kernel & skiboot patches are applied, the result of:
> 
>  $ git diff ~/src/skiboot/include/opal-api.h arch/powerpc/include/asm/opal-api.h
> 
> Should not include anything MPIPL/fadump related.

Sure.

> 
>> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
>> index 57bd029..878110a 100644
>> --- a/arch/powerpc/include/asm/opal.h
>> +++ b/arch/powerpc/include/asm/opal.h
>> @@ -39,6 +39,12 @@ int64_t opal_npu_spa_clear_cache(uint64_t phb_id, uint32_t bdfn,
>>  				uint64_t PE_handle);
>>  int64_t opal_npu_tl_set(uint64_t phb_id, uint32_t bdfn, long cap,
>>  			uint64_t rate_phys, uint32_t size);
>> +
>> +int64_t opal_mpipl_update(enum opal_mpipl_ops op, u64 src,
>> +			  u64 dest, u64 size);
>> +int64_t opal_mpipl_register_tag(enum opal_mpipl_tags tag, uint64_t addr);
>> +int64_t opal_mpipl_query_tag(enum opal_mpipl_tags tag, uint64_t *addr);
>> +
> 
> Please consistently use kernel types for new prototypes in here.

uint64_t instead of 'enum's?

- Hari