On Mon, May 28, 2012 at 1:34 PM, Avi Kivity <a...@redhat.com> wrote: > On 05/24/2012 10:58 PM, Max Filippov wrote: >> On Thu, May 24, 2012 at 6:26 PM, Avi Kivity <a...@redhat.com> wrote: >>> On 05/24/2012 05:11 PM, Max Filippov wrote: >>>>> >>>>> Not in breakpoint_invalidate as the missing offset was compensated >>>>> before your commit (well, starting with c2f07f81a2 in fact). >>>> >>>> I'd say that compensation that you mention >>>> >>>> ram_addr = (memory_region_get_ram_addr(section.mr) >>>> + section.offset_within_region) & TARGET_PAGE_MASK; >>>> this >>>> ram_addr |= (pc & ~TARGET_PAGE_MASK); >>>> tb_invalidate_phys_page_range(ram_addr, ram_addr + 1, 0); >>>> >>>> was removed by f3705d53296d, not by 1e7855a558 >>> >>> Indeed. Note how the |= cleverly accommodates both truncating and >>> non-truncating cpu_get_phys_page_debug(). >> >> Right. If the fix is going to be checked in then TeLeMan's original version >> with '|' is preferable for this reason. > > I disagree. Whatever we call cpu_get_phys_page_debug() has to either > mask out the low bits, or not (I prefer the latter, since it's > unambiguous for large pages), but it has to be consistent. Once it's > consistent, there's no reason to use clever tricks.
I meant a one line fix for the 1.1. I suspect that fixing entire cpu_get_phys_page_debug thing for 1.1 is too risky. -- Thanks. -- Max
