Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 20 Jan 2022 13:13:47 -0800
From:   Josh Poimboeuf <jpoimboe@redhat.com>
To:     "Kirill A. Shutemov" <kirill@shutemov.name>
Cc:     bp@alien8.de, aarcange@redhat.com, ak@linux.intel.com,
        dan.j.williams@intel.com, dave.hansen@intel.com, david@redhat.com,
        hpa@zytor.com, jgross@suse.com, jmattson@google.com,
        joro@8bytes.org, kirill.shutemov@linux.intel.com,
        knsathya@kernel.org, linux-kernel@vger.kernel.org, luto@kernel.org,
        mingo@redhat.com, pbonzini@redhat.com, peterz@infradead.org,
        sathyanarayanan.kuppuswamy@linux.intel.com, sdeep@vmware.com,
        seanjc@google.com, tglx@linutronix.de, tony.luck@intel.com,
        vkuznets@redhat.com, wanpengli@tencent.com, x86@kernel.org
Subject: Re: [PATCH 2/3] x86/boot: Allow to hook up alternative port I/O
 helpers
Message-ID: <20220120211347.6gglputnh7n3wbvw@treble>
References: <Yehz3eqq670WRVJE@zn.tnic>
 <20220120021545.7786-1-kirill.shutemov@linux.intel.com>
 <20220120021545.7786-2-kirill.shutemov@linux.intel.com>
 <20220120163826.bits6ffbnbal4yse@box.shutemov.name>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20220120163826.bits6ffbnbal4yse@box.shutemov.name>
Precedence: bulk

On Thu, Jan 20, 2022 at 07:38:26PM +0300, Kirill A. Shutemov wrote:
> On Thu, Jan 20, 2022 at 05:15:43AM +0300, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/boot/io.h b/arch/x86/boot/io.h
> > new file mode 100644
> > index 000000000000..640daa3925fb
> > --- /dev/null
> > +++ b/arch/x86/boot/io.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef BOOT_IO_H
> > +#define BOOT_IO_H
> > +
> > +#include <asm/shared/io.h>
> > +
> > +struct port_io_ops {
> > +	unsigned char (*inb)(int port);
> > +	unsigned short (*inw)(int port);
> > +	unsigned int (*inl)(int port);
> > +	void (*outb)(unsigned char v, int port);
> > +	void (*outw)(unsigned short v, int port);
> > +	void (*outl)(unsigned int v, int port);
> > +};
> > +
> > +extern struct port_io_ops pio_ops;
> > +
> > +static inline void init_io_ops(void)
> > +{
> > +	pio_ops = (struct port_io_ops){
> > +		.inb = inb,
> > +		.inw = inw,
> > +		.inl = inl,
> > +		.outb = outb,
> > +		.outw = outw,
> > +		.outl = outl,
> > +	};
> > +}
> > +
> > +#endif
> 
> It works fine on x86-64, but breaks on i386:
> 
> ld: Unexpected run-time relocations (.rel) detected!
> 
> I'll change it to
> 
> 	pio_ops.inb = inb;
> 	pio_ops.inw = inw;
> 	pio_ops.inl = inl;
> 	pio_ops.outb = outb;
> 	pio_ops.outw = outw;
> 	pio_ops.outl = outl;
> 
> It works, but I hate that I don't really have control here. I have no clue
> why compiler generate different code after the change. It is very fragile.
> 
> Do we really have no way to say compiler to avoid relactions here?

This one:

	pio_ops = (struct port_io_ops){
		.inb = inb,
		.inw = inw,
		.inl = inl,
		.outb = outb,
		.outw = outw,
		.outl = outl,
	};

.. actually allocates an anonymous struct in the .data section, which is
memcpy'ed at runtime when the assignment occurs.  That anonymous struct
has .data -> .text relocations which have to be resolved at runtime
because the distance between .data and .text isn't constant.

The working version:

 	pio_ops.inb = inb;
 	pio_ops.inw = inw;
 	pio_ops.inl = inl;
 	pio_ops.outb = outb;
 	pio_ops.outw = outw;
 	pio_ops.outl = outl;

... only needs .text -> .text relocations which can be resolved at link
time.

-- 
Josh