Date: Sun, 10 Jan 2021 17:56:26 -0400 From: Mitchell Horne <mhorne@freebsd.org> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org> Subject: Re: git: facdd1cd2045 - main - cgem: add 64-bit support Message-ID: <CADeAsy2hh3H5=qYq91XRaEh6SAMssoKY-1v344bF7ytz%2BcRX-w@mail.gmail.com> In-Reply-To: <8EE9AF5A-E74C-470E-B3E5-A347E3367777@freebsd.org> References: <202101102053.10AKrGqv070360@gitrepo.freebsd.org> <B11437E6-5865-460B-92A0-62D82F2BBEFC@freebsd.org> <X/tyGljg0gdzab2K@kib.kiev.ua> <51C7F0DA-0EA2-45EF-A817-53984205C648@freebsd.org> <X/t0jZIB8rUF1yYH@kib.kiev.ua> <8EE9AF5A-E74C-470E-B3E5-A347E3367777@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jan 10, 2021 at 5:44 PM Jessica Clarke <jrtc27@freebsd.org> wrote:
>
> On 10 Jan 2021, at 21:41, Konstantin Belousov <kostikbel@gmail.com> wrote:
> > On Sun, Jan 10, 2021 at 09:33:16PM +0000, Jessica Clarke wrote:
> >> On 10 Jan 2021, at 21:31, Konstantin Belousov <kostikbel@gmail.com> wrote:
> >>> On Sun, Jan 10, 2021 at 09:17:48PM +0000, Jessica Clarke wrote:
> >>>> On 10 Jan 2021, at 20:53, Mitchell Horne <mhorne@FreeBSD.org> wrote:
> >>>>> diff --git a/sys/dev/cadence/if_cgem.c b/sys/dev/cadence/if_cgem.c
> >>>>> index 3c5277452469..77337e977dcc 100644
> >>>>> --- a/sys/dev/cadence/if_cgem.c
> >>>>> +++ b/sys/dev/cadence/if_cgem.c
> >>>>> @@ -77,6 +77,14 @@ __FBSDID("$FreeBSD$");
> >>>>> #include <dev/mii/mii.h>
> >>>>> #include <dev/mii/miivar.h>
> >>>>>
> >>>>> +#ifdef EXT_RESOURCES
> >>>>> +#include <dev/extres/clk/clk.h>
> >>>>> +#endif
> >>>>> +
> >>>>> +#if INTPTR_MAX == INT64_MAX
> >>>>> +#define CGEM64
> >>>>> +#endif
> >>>>
> >>>> This isn't going to work with CHERI. Perhaps a BUS_SPACE_MAXADDR check,
> >>>> or SIZE_MAX? The former is probably preferable for PAE systems.
> >>>
> >>> Wouldn't it be spelled as __LP64__ ?
> >>
> >> It shouldn't be, because the P stands for pointer(s), which we make
> >> 128-bit, though in practice for compatibility we have a hack to define
> >> it anyway (but the compiler *won't*, it's in bsd.cpu.mk or similar).
> >> Only the integer addresses are 128-bit.
> >
> > I have no idea about CHERI, I mean how this statement should be handled
> > for FreeBSD.
>
> Well, ideally in a way that works for FreeBSD and CHERI. This one
> happens to do that, though it doesn't feel very idiomatic, and a
> comparison against SIZE_MAX or BUS_SPACE_MAXADDR would seem more
> accurate (depending on the behaviour you want for PAE systems). FreeBSD
> *can* use __LP64__ but it causes pain for us downstream so we prefer
> people to be more specific in what they mean (i.e., do you care about
> the size of the address space or the size of the representation of
> pointers).
>
> Jess
>
Indeed, the committed version is not very idiomatic. In this case I
would think that a comparison like `#if BUS_SPACE_MAXADDR >
BUS_SPACE_MAXADDR_32BIT` describes the intent, but I don't see
anything like that in the tree already. I do see several occurrences
of `#ifdef __LP64__` under sys/dev/.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CADeAsy2hh3H5=qYq91XRaEh6SAMssoKY-1v344bF7ytz%2BcRX-w>