On Tue, 5 May 2020 at 15:04, Richard Henderson <richard.hender...@linaro.org> wrote: > > On 5/5/20 2:49 AM, Peter Maydell wrote: > > On Mon, 4 May 2020 at 17:03, Richard Henderson > > <richard.hender...@linaro.org> wrote: > >> > >> On 5/4/20 2:43 AM, Peter Maydell wrote: > >>> I've reviewed patch 13, but I still don't understand why you've > >>> made the size-related changes in patch 4, so I've continued > >>> our conversation in the thread on the v3 version of that patch. > >> > >> I've changed that here in v4. Please have another look at this one. > > > > The page_check_range() call still seems to be passing a fixed > > size of '1' ? > > We only need to validate one page, so validating one byte on the page is > sufficient. The size argument to page_check_range is so that it can validate > multiple pages at a time.
Yes, but why *change* what we're doing? If the patch doesn't change details like this then I can review it by looking at it as a code refactor. If it changes this sort of thing then now I have to look into the details of what exactly page_check_range() is doing and whether it's possible to get here for an access that crosses a page boundary, and the review job gets harder. If passing 1 rather than the actual size we have is the right thing, then split that into its own patch with its own commit message that can explain why it needs to be changed. If it doesn't matter whether we pass 1 or size, then please don't change it in a refactoring patch. thanks -- PMM
