svn commit: r327950 - in head/sys/powerpc: aim include powerpc ps3

Nathan Whitehorn nwhitehorn at freebsd.org
Tue Jan 16 02:40:48 UTC 2018



On 01/15/18 15:42, Konstantin Belousov wrote:
> On Mon, Jan 15, 2018 at 03:20:49PM -0800, Nathan Whitehorn wrote:
>> Fair enough. Here's a patch with a new flag (DIRECT_MAP_AVAILABLE). I've
>> also retooled the sfbuf code to use this rather than its own flags that
>> mean the same things. The sparc64 part of the patch is untested.
>> -Nathan
>> Index: amd64/include/vmparam.h
>> ===================================================================
>> --- amd64/include/vmparam.h	(revision 328006)
>> +++ amd64/include/vmparam.h	(working copy)
>> @@ -190,6 +190,7 @@
>>    * because the result is not actually accessed until later, but the early
>>    * vt fb startup needs to be reworked.
>>    */
>> +#define	DIRECT_MAP_AVAILABLE	1
>>   #define	PHYS_TO_DMAP(x)	({						\
>>   	KASSERT(dmaplimit == 0 || (x) < dmaplimit,			\
>>   	    ("physical address %#jx not covered by the DMAP",		\
>> Index: arm64/include/vmparam.h
>> ===================================================================
>> --- arm64/include/vmparam.h	(revision 328006)
>> +++ arm64/include/vmparam.h	(working copy)
>> @@ -176,6 +176,7 @@
>>   #define	VIRT_IN_DMAP(va)	((va) >= DMAP_MIN_ADDRESS && \
>>       (va) < (dmap_max_addr))
>>   
>> +#define	DIRECT_MAP_AVAILABLE
> Just define, or define it to 1 ?

Yes, sorry for typo.

>
>>   #define	PHYS_TO_DMAP(pa)						\
>>   ({									\
>>   	KASSERT(PHYS_IN_DMAP(pa),					\
>> Index: dev/efidev/efirt.c
>> ===================================================================
>> --- dev/efidev/efirt.c	(revision 328006)
>> +++ dev/efidev/efirt.c	(working copy)
>> @@ -115,6 +115,11 @@
>>   		return (0);
>>   	}
>>   	efi_systbl = (struct efi_systbl *)PHYS_TO_DMAP(efi_systbl_phys);
>> +	if (efi_systbl == NULL) {
>> +		if (bootverbose)
>> +			printf("EFI systbl not mapped in kernel VA\n");
>> +		return (0);
>> +	}
> Is this chunk still needed ?

The existing code is a bit of an awkward superposition of the "return 
NULL" idea and having the flag. Since you think there will never be 
intermediate cases -- which seems reasonable -- I will rip the 
conditional logic out and add a KASSERT matching the ones on arm64 and 
amd64 to the powerpc version.

>
>>   	if (efi_systbl->st_hdr.th_sig != EFI_SYSTBL_SIG) {
>>   		efi_systbl = NULL;
>>   		if (bootverbose)
>> Index: kern/subr_sfbuf.c
>> ===================================================================
>> --- kern/subr_sfbuf.c	(revision 328006)
>> +++ kern/subr_sfbuf.c	(working copy)
>> @@ -88,8 +88,8 @@
>>   	vm_offset_t sf_base;
>>   	int i;
>>   
>> -#ifdef SFBUF_OPTIONAL_DIRECT_MAP
>> -	if (SFBUF_OPTIONAL_DIRECT_MAP)
>> +#ifdef DIRECT_MAP_AVAILABLE
>> +	if (DIRECT_MAP_AVAILABLE)
>>   		return;
> Would it make sense to define the symbol on all other arches as 0 then,
> and remove #ifdef ? Returning to your initial proposal of relying on the
> compiler optimiing if (0) block; out.

That is a good idea.

> Also, just curious, why did you spelled DMAP as DIRECT_MAP ?
>

DMAP without the PHYS_TO_ seemed lacking in context and I was worried 
there might be a collision on DMAP. PMAP_HAS_DMAP would also work; I 
don't have a preference.

Thanks for your patience working this out in real time with me.
-Nathan


More information about the svn-src-head mailing list