Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Nov 2019 23:28:16 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        John Baldwin <jhb@freebsd.org>
Cc:        Li-Wen Hsu <lwhsu@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r354900 - head/usr.sbin/jail
Message-ID:  <CANCZdfqD=upQnLrcNHR=F6gO6Po-_0nyNdn0ymgaEtN-TneoDQ@mail.gmail.com>
In-Reply-To: <991bdc33-516d-6e6d-1880-44930441893d@FreeBSD.org>
References:  <201911201654.xAKGsMTv094014@repo.freebsd.org> <CANCZdfqVLZUqGiCDagwkULH7xegrZwehRsn8Ek-BJR=OVTpXGw@mail.gmail.com> <59bf120c-2f35-1a22-b6fa-a9c9bb8cfdf4@FreeBSD.org> <CANCZdfoG198Qqj90NS=NZ_%2BsMWoEPffGcJJmg25u4U97KgPfBQ@mail.gmail.com> <991bdc33-516d-6e6d-1880-44930441893d@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Nov 20, 2019 at 4:32 PM John Baldwin <jhb@freebsd.org> wrote:

> On 11/20/19 3:04 PM, Warner Losh wrote:
> > On Wed, Nov 20, 2019 at 3:09 PM John Baldwin <jhb@freebsd.org> wrote:
> >
> >> On 11/20/19 10:01 AM, Warner Losh wrote:
> >>> On Wed, Nov 20, 2019 at 9:54 AM Li-Wen Hsu <lwhsu@freebsd.org> wrote:
> >>>
> >>>> Author: lwhsu
> >>>> Date: Wed Nov 20 16:54:21 2019
> >>>> New Revision: 354900
> >>>> URL: https://svnweb.freebsd.org/changeset/base/354900
> >>>>
> >>>> Log:
> >>>>   Use the correct variable, also limit the scope to bfd
> >>>>
> >>>>   PR:           242109
> >>>>   Reported by:  jhb
> >>>>   Sponsored by: The FreeBSD Foundation
> >>>>
> >>>> Modified:
> >>>>   head/usr.sbin/jail/Makefile
> >>>>
> >>>> Modified: head/usr.sbin/jail/Makefile
> >>>>
> >>>>
> >>
> ==============================================================================
> >>>> --- head/usr.sbin/jail/Makefile Wed Nov 20 16:35:58 2019
> >> (r354899)
> >>>> +++ head/usr.sbin/jail/Makefile Wed Nov 20 16:54:21 2019
> >> (r354900)
> >>>> @@ -18,7 +18,7 @@ CFLAGS+=-I. -I${.CURDIR}
> >>>>  # workaround for GNU ld (GNU Binutils) 2.33.1:
> >>>>  #   relocation truncated to fit: R_RISCV_GPREL_I against `.LANCHOR2'
> >>>>  # https://bugs.freebsd.org/242109
> >>>> -.if ${MACHINE_ARCH} == "riscv"
> >>>> +.if ${LINKER_TYPE} == "bfd" && ${MACHINE} == "riscv"
> >>>>
> >>>
> >>> MACHINE isn't the right thing to use here. It's never the proper thing
> in
> >>> userland makefiles, unless they are interfacing with the kernel.
> >>>
> >>> MACHINE_CPUARCH is what you want here.
> >>
> >> Eh, that claim doesn't seem quite true.  src.opts.mk only uses MACHINE
> >> and not
> >> MACHINE_CPUARCH for example (to set _TT that is then used all over the
> >> place in src.opts.mk).  My experience is that uses of *_CPUARCH are in
> >> fact
> >> pretty rare.
> >>
> >
> > However, __TT is used bogusly in many places in src.opts.mk. They are
> all
> > relatively new related to llvm (and one for google test). MACHINE has
> > always been for the kernel and MACHINE_ARCH for userland. MACHINE_CPUARCH
> > was created for those architectures where we have a number of
> MACHINE_ARCH
> > to make things easier to cope with.
> >
> > I've done several sweeps of the tree over the years to keep this
> enforced,
> > so I'm quite sure of the dichotomy...
>
> Here are some to fix then: :)
>
> sbin/reboot/Makefile:.if exists(${.CURDIR}/boot_${MACHINE}.8)
> sbin/reboot/Makefile:MAN+=      boot_${MACHINE}.8
> sbin/reboot/Makefile:MLINKS+= boot_${MACHINE}.8 boot.8
> sbin/reboot/Makefile:.if ${MACHINE} == "amd64"
> usr.sbin/bsdinstall/partedit/Makefile:PARTEDIT_ARCH= ${MACHINE}
> usr.sbin/bsdinstall/partedit/Makefile:.if ${MACHINE} == "i386" ||
> ${MACHINE} == "amd64"
> usr.sbin/pkg/Makefile:.  if ${MACHINE} != "amd64" && ${MACHINE} != "i386"
>

I'm not sure these are wrong... reboot is based on the kernel we're
running, though. partedit is based on what kind of partition scheme the
kernel wants. pkg is just wrong, though.  Traditionally in BSD, different
ways of booting usually went hand and hand with needing different kernels.
Though after BSD passed from mini/micro computers where this was true into
embedded things got blurrier. Likewise with the partitioning schemes: those
used to be different for every type of computer, but now have standardized
around GPT with a few legacy systems like MBR and APM lingering on. BSD
labels were originally invented to have a standard way to label a disk, but
even that grew a lot of variations.


> This one also seems dubious, but in a different way:
>
> usr.bin/Makefile:
>
> # ARM64TODO gprof does not build
> # RISCVTODO gprof does not build
> .if ${MACHINE_ARCH} != "aarch64" && ${MACHINE_CPUARCH} != "riscv"
> SUBDIR.${MK_TOOLCHAIN}+=        gprof
> .endif
>

Yes. That's likely incorrect.
> Somewhat exacerbated by the whole aarch64 vs arm64 thing and probably
confusion on when to use CPUARCH vs ARCH.

When I invented CPUARCH, I didn't make it as clean as I'd like. It's always
for 'what directory do we use in the tree' and is also convenient for other
things. It was thought you'd use CPUARCH when you are testing generically
for an arch. You'd use MACHINE_ARCH when you needed a specific one (so both
of these should be MACHINE_CPUARCH). The notion was you'd only need to use
MACHINE_ARCH rarely in ifdefs and usually only inside of system-wide .mk
files.


> BTW, MACHINE_ARCH seems to matter just as much for the kernel.  64-bit
> mips runs a "mips64" kernel, not a "mips" kernel.
>

It does.

With PC-98 removed, I don't think we have any cases where MACHINE !=
> MACHINE_CPUARCH now?
>

Well, there's arm64 / aarch64.

I'm not at all opposed to changing the definitions, but they are what I've
been saying. I've done a lot of cleanup of mis-uses over the last 10 or 15
years, so these characterizations are correct. I'll admit I've done not the
best job at documenting it, though. Coming up with new definitions is
fraught, I'd think, since it's easy to have a notion of what the right
thing is, but harder to cast it into good prose that's clear to a wide
audience.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqD=upQnLrcNHR=F6gO6Po-_0nyNdn0ymgaEtN-TneoDQ>