Re: radeon, amdgpu improvements for aarch64

On Thu, 28 Dec 2023 00:19:44 +0000
Taylor R Campbell <[email protected]> wrote:

> > +#ifdef _LP64
> > +	if (eaddr < (1UL<<40)) {

> Just to confirm: eaddr is inclusive here, right?  As in, if the range
> were [0x10000, 0x20000), we would have saddr=0x10000 eaddr=0x1ffff (or
> maybe eaddr=0x1f000), right?

The end boundary is page-aligned non-inclusive. Thus strict less-than
is correct. I'll change to ((paddr_t)1 << 40) for clarity.

> >  #if defined(__i386__) || defined(__x86_64__)
> >  	return x86_select_freelist(tag->_bounce_alloc_hi - 1);
> > +#elif defined(__aarch64__)
> > +	return VM_FREELIST_FIRST1T;
> 
> This should look through the tag->_ranges to choose
> VM_FREELIST_FIRST1T if it has been restricted with
> bus_dmatag_subregion to lie in that range, and VM_FREELIST_DEFAULT if
> not.

Understood. Will try to implement this.

> > --- sys/uvm/uvm_pglist.c	21 Dec 2021 08:27:49 -0000	1.90
> > +++ sys/uvm/uvm_pglist.c	27 Dec 2023 22:06:10 -0000
> > @@ -112,8 +112,9 @@ static int
> >  uvm_pglistalloc_c_ps(uvm_physseg_t psi, int num, paddr_t low, paddr_t high,
> >      paddr_t alignment, paddr_t boundary, struct pglist *rlist)
> >  {
> > -	signed int candidate, limit, candidateidx, end, idx, skip;
> > -	int pagemask;
> > +	long candidate, limit, candidateidx, end, idx;
> > +	int skip;
> > +	long pagemask;
> 
> I don't really have an issue with this but I think we may need to
> switch int to pfn_t for page frame numbers much more systematically.
> Curious how changing only start_hint from int to long helps?  (Commit
> message doesn't explain.)

There's some details in PR 57683.
Basically physseg's avil_start and avail_end are already 64-bit (pfn
encoded in paddr_t) but the hint is for some reason 32-bit so it can
overflow leading to an assertion. I agree it would be better to use
pfn_t consistently but that's a larger scope change than avoiding this
specific overflow issue. Since the code uses uimax() alot, extending
to u_long and ulmax() for now seemed most natural.