Re: [PATCH v3 1/3] target/arm: Add support for FEAT_TLBIRANGE

[Richard Henderson]

On 3/15/21 12:34 PM, Rebecca Cran wrote:
> On 3/10/21 12:24 PM, Richard Henderson wrote:
> > On 3/9/21 6:29 PM, Rebecca Cran wrote:
> > > +void tlb_flush_page_range_by_mmuidx(CPUState *cpu, target_ulong addr,
> > > +                                    unsigned int num_pages, uint16_t idxmap)
> >
> > I am not keen on this interface.  I think you should take either start+end 
> > addresses (inclusive) or start+length (in bytes).
> >
> > Using num_pages, and as an unsigned int, seems too easy to fail when applied 
> > to a different guest.
>
> Do you mean pushing the knowledge of the number of pages to invalidate down to 
> cputlb.c?

Yes.

In particular, your interface does not allow a single call to invalidate 1/2 of 
the total address space.  Because the type for num_pages isn't large enough.

There's nothing else in the cputlb interface that is page-based, except for 
"flush one page", and I thought that either

  target_ulong addr, target_ulong length, unsigned bits

would be a clearer interface to use.

> Because I'm thinking there has to be a loop somewhere that 
> invalidates each page if a full flush isn't being done?

Certainly.

r~