[PATCH v1 0/2] [libbsd] Install correct machine include headers

[Chris Johns]

On 2/6/21 4:20 am, Christian Mauderer wrote:
> On 01/06/2021 19:24, Gedare Bloom wrote:
>> On Mon, May 10, 2021 at 11:26 AM Jan Sommer <jan.sommer at dlr.de> wrote:
>>>
>>> Hello,
>>>
>>> This is a follow-up on this discussion regarding the installed header files
>>> in libbsd: https://lists.rtems.org/pipermail/devel/2021-April/066211.html
>>>
>>> The current situation is, that for example for all machines the bus.h is
>>> installed from
>>> within the rtemsbsd directory
>>> (https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/machine/bus.h).
>>>
>>> According to the file docu it originates from the amd64 version of this file.
>>> It also has the following section in it which we ran into when compiling
>>> Chris' ptpd2 archive:
>>>
>>> #ifdef __i386__
>>>    #error "your include paths are wrong"
>>> #endif
>>>
>>> This patchset does the following:
>>> - Add the target dependent machine include directory to 'header-paths' in
>>> libbsd.py
>>> - Import (mostly) '_bus.h', 'bus.h' and 'cpufunc.h' for the targets from
>>> freebsd-org
>>> - Remove those header files from rtemsbsd directory
>>>
>>> As a result the matching versions for machine dependent header files are now
>>> installed
>>> for each BSP.
>>> Would this be an acceptable solution?
>>>
>>> So far I compiled some BSPs for i386, arm, aarch64, powerpc, riscv, sparc and
>>> sparc64 to check that they still compile after the changes.
>>> Are there any other architectures which should be included?
> 
> I think the best starting point to find out the minimum supported platforms is
> the nexus-devices.h. At least the officially supported BSPs should have an entry
> there. That is:
> 
> #if defined(LIBBSP_ARM_REALVIEW_PBX_A9_BSP_H)
> #elif defined(LIBBSP_ARM_BEAGLE_BSP_H)
> #elif defined(LIBBSP_ARM_LPC32XX_BSP_H)
> #elif defined(LIBBSP_M68K_GENMCF548X_BSP_H)
> #elif defined(LIBBSP_ARM_XILINX_ZYNQ_BSP_H)
> #elif defined(LIBBSP_AARCH64_XILINX_ZYNQMP_BSP_H)
> #elif defined(LIBBSP_ARM_ATSAM_BSP_H)
> #elif defined(LIBBSP_ARM_ALTERA_CYCLONE_V_BSP_H)
> #elif defined(LIBBSP_ARM_IMX_BSP_H)
> #elif defined(LIBBSP_ARM_IMXRT_BSP_H)
> #elif defined(LIBBSP_ARM_LPC24XX_BSP_H)
> #elif defined(LIBBSP_ARM_STM32H7_BSP_H)
> #elif defined(LIBBSP_I386_PC386_BSP_H)
> #elif defined(LIBBSP_POWERPC_QORIQ_BSP_H)
> #elif defined(LIBBSP_POWERPC_TQM8XX_BSP_H)
> #elif defined(LIBBSP_POWERPC_MOTOROLA_POWERPC_BSP_H)
> 
> So I think you should have a look at m68k too.
> 
> Did you try to run it on any of the platforms?
> 
>>>
>>> I ran into one problem regarding the compilation of
>>> rtemsbsd/sys/dev/dw_mmc/dw_mmc.c:105
>>>
>>>> return (bus_space_read_4(0, sc->bushandle, off));
>>>
>>> The first parameter creates an error for riscv (I think because dereferencing
>>> a NULL pointer).
>>> Are there any suggestion how to solve it (I am not familiar with the bus
>>> space and there is a lot of macro magic going on)?
>>>
>> It looks like this will be a problem for any architecture. so should
>> the function that calls bus_space_write_4(0 ...)
>>   The macro trail goes...
>>
>> #define        __bs_rs(sz, t, h, o)                                            \
>>         (*(t)->__bs_opname(r,sz))((t)->bs_cookie, h, o)
>>
>> #define        bus_space_read_4(t, h, o)       __bs_rs(4,(t),(h),(o))
>>
>> so t is dereferenced. It appears to be an error in the API usage by
>> the dw_mmc.c code to not specify a valid bus space.
> 
> dw_mmc is only relevant for very few platforms. So in theory it could be limited
> to these. But most likely that won't really solve the problem.
> 
> The right answer is that dw_mmc doesn't use the API like intended (like Gedare
> said). That could be a problem for more drivers that use stuff from bus.h in
> rtemsbsd. They never had to use that API correctly. Even worse: Some of the
> __rtems__ hacks in freebsd could have that problem too.
> 
> I think for this patch set we either need to have a thorough look at these
> locations or run (not only build) it on a number of platforms, especially the
> ones with special drivers in rtemsbsd or with heavily adapted drivers.

The only arch that supports a tag we current have is the x86 and that is for IO
vs mem space. I posted some patches earlier this year I need to revisit for the
powerpc (mvme2700). If possible we prefer no tags and a single flat address
space that is cache coherent. In the x86 tags cannot be avoided and supported
drivers handle the tags correctly.

Be-careful reviewing FreeBSD's bus space implementations. For example the
PowerPC support uses tables of calls and we do not need that overhead.

The issue here is using this driver on the x86 platform. Until that happens I am
fine with the tag being 0. The API requires something even if it is not used.

Chris