[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [Patch v3] x86/traps: improvements to {rd, wr}msr_hypervisor_regs()
>>> On 07.10.13 at 15:15, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote: >>> - switch ( idx ) >>> + switch ( idx - base ) >>> { >>> - case 0: >>> + case 0: /* Write hypercall page */ >>> { >>> void *hypercall_page; >>> - unsigned long gmfn = val >> 12; >>> - unsigned int idx = val & 0xfff; >>> + unsigned long gmfn = val >> PAGE_SHIFT; >>> struct page_info *page; >>> p2m_type_t t; >>> >>> - if ( idx > 0 ) >>> + if ( val & 0xfff ) >> If you're not going to use 12, then shouldn't you use PAGE_SIZE-1 rather >> than 0xfff? >> >>> { >>> + /* Bottom 12 bits are hypercall page index. Only 0 is valid. >>> */ >> And modify this comment accordingly? > > I suppose. The use of ~PAGE_MASK before was my mistaken impression that > this was an alignment check rather than a hypercall index check. > > As this stuff is not documented anywhere I can find (other than by > reading the code), I am not sure it is caring too much about. Ideally, > there would be somewhere XEN_MSR_HYPERCALL_PAGE_INDEX_MASK as 0xfff and > XEN_MSR_HYPERCALL_PAGE_MAX_INDEX, but as it is unlikely that Xen will > ever start using multiple hypercall pages, this sounds like overkill. > > I am happy to implement it however people prefer, but would err on the > lazy side of leaving it alone if there is no overwhelming objection. As Paul said - if you use PAGE_SHIFT, you also should replace 0xfff. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |