| Message ID | 57B6D6C40200007800107465@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/08/16 08:52, Jan Beulich wrote: > Page tables get pre-populated with physical addresses which, due to > living inside the Xen image, will never exceed 32 bits in width. That > in turn results in the tool generating the relocations to produce > 32-bit relocations for them instead of the 64-bit ones needed for > relocating virtual addresses. Hence instead of special casing page > tables in the processing of 64-bit relocations, let's be more rigid > and refuse them (as being indicative of something else having gone > wrong in the build process). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Is it an ABI requirement to use the minimal available relocation? It is certainly suboptimal to use a 64bit relocation when a 32bit one would do, but I wouldn't bet that it is unconditional avoided by all toolchains. It is currently the case that Xen needs to live below 4GB physical, so from that point of view a 64bit relocation will not be required in the pagetables. ~Andrew > > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -81,12 +81,9 @@ static void __init efi_arch_relocate_ima > } > break; > case PE_BASE_RELOC_DIR64: > - if ( delta ) > - { > - *(u64 *)addr += delta; > - if ( in_page_tables(addr) ) > - *(intpte_t *)addr += xen_phys_start; > - } > + if ( in_page_tables(addr) ) > + blexit(L"Unexpected relocation type"); > + *(u64 *)addr += delta; > break; > default: > blexit(L"Unsupported relocation type"); > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
>>> On 19.08.16 at 14:39, <andrew.cooper3@citrix.com> wrote: > On 19/08/16 08:52, Jan Beulich wrote: >> Page tables get pre-populated with physical addresses which, due to >> living inside the Xen image, will never exceed 32 bits in width. That >> in turn results in the tool generating the relocations to produce >> 32-bit relocations for them instead of the 64-bit ones needed for >> relocating virtual addresses. Hence instead of special casing page >> tables in the processing of 64-bit relocations, let's be more rigid >> and refuse them (as being indicative of something else having gone >> wrong in the build process). >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Is it an ABI requirement to use the minimal available relocation? It is > certainly suboptimal to use a 64bit relocation when a 32bit one would > do, but I wouldn't bet that it is unconditional avoided by all toolchains. What ABI? The tool in question is one of our own making. And the way relocations get generated it's hard to tell those that have to be 32-bit (in early boot code and trampoline code) from those that may as well be 64-bit ones (in page tables). > It is currently the case that Xen needs to live below 4GB physical, so > from that point of view a 64bit relocation will not be required in the > pagetables. And even if Xen didn't itself have this requirement, the EFI loader would always put us below 4Gb. Jan
>>> On 19.08.16 at 09:52, <JBeulich@suse.com> wrote: > Page tables get pre-populated with physical addresses which, due to > living inside the Xen image, will never exceed 32 bits in width. That > in turn results in the tool generating the relocations to produce > 32-bit relocations for them instead of the 64-bit ones needed for > relocating virtual addresses. Hence instead of special casing page > tables in the processing of 64-bit relocations, let's be more rigid > and refuse them (as being indicative of something else having gone > wrong in the build process). > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Ping? > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -81,12 +81,9 @@ static void __init efi_arch_relocate_ima > } > break; > case PE_BASE_RELOC_DIR64: > - if ( delta ) > - { > - *(u64 *)addr += delta; > - if ( in_page_tables(addr) ) > - *(intpte_t *)addr += xen_phys_start; > - } > + if ( in_page_tables(addr) ) > + blexit(L"Unexpected relocation type"); > + *(u64 *)addr += delta; > break; > default: > blexit(L"Unsupported relocation type");
On 19/08/16 13:57, Jan Beulich wrote: >>>> On 19.08.16 at 14:39, <andrew.cooper3@citrix.com> wrote: >> On 19/08/16 08:52, Jan Beulich wrote: >>> Page tables get pre-populated with physical addresses which, due to >>> living inside the Xen image, will never exceed 32 bits in width. That >>> in turn results in the tool generating the relocations to produce >>> 32-bit relocations for them instead of the 64-bit ones needed for >>> relocating virtual addresses. Hence instead of special casing page >>> tables in the processing of 64-bit relocations, let's be more rigid >>> and refuse them (as being indicative of something else having gone >>> wrong in the build process). >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> Is it an ABI requirement to use the minimal available relocation? It is >> certainly suboptimal to use a 64bit relocation when a 32bit one would >> do, but I wouldn't bet that it is unconditional avoided by all toolchains. > What ABI? The tool in question is one of our own making. And the > way relocations get generated it's hard to tell those that have to > be 32-bit (in early boot code and trampoline code) from those that > may as well be 64-bit ones (in page tables). > >> It is currently the case that Xen needs to live below 4GB physical, so >> from that point of view a 64bit relocation will not be required in the >> pagetables. > And even if Xen didn't itself have this requirement, the EFI loader > would always put us below 4Gb. Why is this necessarily true? xen.efi is built as a 64bit PE, not 32bit. ~Andrew
>>> On 30.08.16 at 18:35, <andrew.cooper3@citrix.com> wrote: > On 19/08/16 13:57, Jan Beulich wrote: >>>>> On 19.08.16 at 14:39, <andrew.cooper3@citrix.com> wrote: >>> On 19/08/16 08:52, Jan Beulich wrote: >>>> Page tables get pre-populated with physical addresses which, due to >>>> living inside the Xen image, will never exceed 32 bits in width. That >>>> in turn results in the tool generating the relocations to produce >>>> 32-bit relocations for them instead of the 64-bit ones needed for >>>> relocating virtual addresses. Hence instead of special casing page >>>> tables in the processing of 64-bit relocations, let's be more rigid >>>> and refuse them (as being indicative of something else having gone >>>> wrong in the build process). >>>> >>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> Is it an ABI requirement to use the minimal available relocation? It is >>> certainly suboptimal to use a 64bit relocation when a 32bit one would >>> do, but I wouldn't bet that it is unconditional avoided by all toolchains. >> What ABI? The tool in question is one of our own making. And the >> way relocations get generated it's hard to tell those that have to >> be 32-bit (in early boot code and trampoline code) from those that >> may as well be 64-bit ones (in page tables). >> >>> It is currently the case that Xen needs to live below 4GB physical, so >>> from that point of view a 64bit relocation will not be required in the >>> pagetables. >> And even if Xen didn't itself have this requirement, the EFI loader >> would always put us below 4Gb. > > Why is this necessarily true? > > xen.efi is built as a 64bit PE, not 32bit. The file format doesn't matter here. And we'd have other problems to solve if we got loaded above 4Gb. Jan
On 31/08/16 13:28, Jan Beulich wrote: >>>> On 30.08.16 at 18:35, <andrew.cooper3@citrix.com> wrote: >> On 19/08/16 13:57, Jan Beulich wrote: >>>>>> On 19.08.16 at 14:39, <andrew.cooper3@citrix.com> wrote: >>>> On 19/08/16 08:52, Jan Beulich wrote: >>>>> Page tables get pre-populated with physical addresses which, due to >>>>> living inside the Xen image, will never exceed 32 bits in width. That >>>>> in turn results in the tool generating the relocations to produce >>>>> 32-bit relocations for them instead of the 64-bit ones needed for >>>>> relocating virtual addresses. Hence instead of special casing page >>>>> tables in the processing of 64-bit relocations, let's be more rigid >>>>> and refuse them (as being indicative of something else having gone >>>>> wrong in the build process). >>>>> >>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>> Is it an ABI requirement to use the minimal available relocation? It is >>>> certainly suboptimal to use a 64bit relocation when a 32bit one would >>>> do, but I wouldn't bet that it is unconditional avoided by all toolchains. >>> What ABI? The tool in question is one of our own making. And the >>> way relocations get generated it's hard to tell those that have to >>> be 32-bit (in early boot code and trampoline code) from those that >>> may as well be 64-bit ones (in page tables). >>> >>>> It is currently the case that Xen needs to live below 4GB physical, so >>>> from that point of view a 64bit relocation will not be required in the >>>> pagetables. >>> And even if Xen didn't itself have this requirement, the EFI loader >>> would always put us below 4Gb. >> Why is this necessarily true? >> >> xen.efi is built as a 64bit PE, not 32bit. > The file format doesn't matter here. And we'd have other problems > to solve if we got loaded above 4Gb. Right, but that doesn't prohibit the generation of 64bit relocations in the pagetables. This patch still seems conceptually incorrect. ~Andrew
>>> On 31.08.16 at 14:32, <andrew.cooper3@citrix.com> wrote: > On 31/08/16 13:28, Jan Beulich wrote: >>>>> On 30.08.16 at 18:35, <andrew.cooper3@citrix.com> wrote: >>> On 19/08/16 13:57, Jan Beulich wrote: >>>>>>> On 19.08.16 at 14:39, <andrew.cooper3@citrix.com> wrote: >>>>> On 19/08/16 08:52, Jan Beulich wrote: >>>>>> Page tables get pre-populated with physical addresses which, due to >>>>>> living inside the Xen image, will never exceed 32 bits in width. That >>>>>> in turn results in the tool generating the relocations to produce >>>>>> 32-bit relocations for them instead of the 64-bit ones needed for >>>>>> relocating virtual addresses. Hence instead of special casing page >>>>>> tables in the processing of 64-bit relocations, let's be more rigid >>>>>> and refuse them (as being indicative of something else having gone >>>>>> wrong in the build process). >>>>>> >>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>>>> Is it an ABI requirement to use the minimal available relocation? It is >>>>> certainly suboptimal to use a 64bit relocation when a 32bit one would >>>>> do, but I wouldn't bet that it is unconditional avoided by all toolchains. >>>> What ABI? The tool in question is one of our own making. And the >>>> way relocations get generated it's hard to tell those that have to >>>> be 32-bit (in early boot code and trampoline code) from those that >>>> may as well be 64-bit ones (in page tables). >>>> >>>>> It is currently the case that Xen needs to live below 4GB physical, so >>>>> from that point of view a 64bit relocation will not be required in the >>>>> pagetables. >>>> And even if Xen didn't itself have this requirement, the EFI loader >>>> would always put us below 4Gb. >>> Why is this necessarily true? >>> >>> xen.efi is built as a 64bit PE, not 32bit. >> The file format doesn't matter here. And we'd have other problems >> to solve if we got loaded above 4Gb. > > Right, but that doesn't prohibit the generation of 64bit relocations in > the pagetables. How that? As already said, we produce the relocations by a tool of our own, and that tool together with how the inputs passed to it get built preclude 64-bit relocations in page tables (or else our image size would need to come close to or exceed 4Gb). Jan
--- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -81,12 +81,9 @@ static void __init efi_arch_relocate_ima } break; case PE_BASE_RELOC_DIR64: - if ( delta ) - { - *(u64 *)addr += delta; - if ( in_page_tables(addr) ) - *(intpte_t *)addr += xen_phys_start; - } + if ( in_page_tables(addr) ) + blexit(L"Unexpected relocation type"); + *(u64 *)addr += delta; break; default: blexit(L"Unsupported relocation type");