On Sat, Nov 30, 2019 at 01:13:10AM +0200, Jarkko Sakkinen wrote:
> +static bool __init sgx_alloc_epc_section(u64 addr, u64 size,
> + unsigned long index,
> + struct sgx_epc_section *section)
> +{
> + unsigned long nr_pages = size >> PAGE_SHIFT;
I'm assuming here that size which gets communicated through CPUID -
which is an interesting way to communicate SGX settings in itself :-) - is
in multiples of 4K? SDM doesn't say...
And last time I asked:
"This size comes from CPUID but it might be prudent to sanity-check it
nevertheless, before doing the memremap()."
but it was left uncommented.
> +/**
> + * A section metric is concatenated in a way that @low bits 12-31 define the
> + * bits 12-31 of the metric and @high bits 0-19 define the bits 32-51 of the
> + * metric.
> + */
> +static inline u64 __init sgx_calc_section_metric(u64 low, u64 high)
> +{
> + return (low & GENMASK_ULL(31, 12)) +
> + ((high & GENMASK_ULL(19, 0)) << 32);
> +}
> +
> +static bool __init sgx_page_cache_init(void)
> +{
> + u32 eax, ebx, ecx, edx, type;
> + u64 pa, size;
> + int i;
> +
> + BUILD_BUG_ON(SGX_MAX_EPC_SECTIONS > (SGX_EPC_SECTION_MASK + 1));
> +
> + for (i = 0; i < (SGX_MAX_EPC_SECTIONS + 1); i++) {
Those brackets are still here from the last time. You said:
"For nothing :-)
I'll change it as:
for (i = 0; i <= SGX_MAX_EPC_SECTIONS; i++) {"
but probably forgot...
and looking at my review comments here:
https://lkml.kernel.org/r/20191005092627.GA25699@xxxxxxx
and your reply:
https://lkml.kernel.org/r/20191007115850.GA20830@xxxxxxxxxxxxxxx
you clearly missed addressing some so I'm going to stop reviewing here.
Please have a look at those review comments again and check whether the
apply - and then do them - or they don't and they pls explain why they
don't.
And do that for the rest of the patchset, please, before you send it
again.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
