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:   Wed, 11 Aug 2021 10:20:28 +0300
From:   Mike Rapoport <rppt@kernel.org>
To:     Christoph Hellwig <hch@infradead.org>
Cc:     Konrad Rzeszutek Wilk <konrad@darnok.org>,
        Maurizio Lombardi <mlombard@redhat.com>, bp@alien8.de,
        tglx@linutronix.de, x86@kernel.org, pjones@redhat.com,
        konrad@kernel.org, george.kennedy@oracle.com, rafael@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH V3] iscsi_ibft: fix crash due to KASLR physical memory
 remapping
Message-ID: <YRN6PN9uZeauadRJ@kernel.org>
References: <20210729135250.32212-1-mlombard@redhat.com>
 <YRKwqAlgWVGVpEJv@infradead.org>
 <YRK9fxhyPgEzWKce@localhost.localdomain>
 <YRN01YySPVucdCF0@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YRN01YySPVucdCF0@infradead.org>
Precedence: bulk

On Wed, Aug 11, 2021 at 07:57:25AM +0100, Christoph Hellwig wrote:
> On Tue, Aug 10, 2021 at 01:55:11PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Tue, Aug 10, 2021 at 06:00:24PM +0100, Christoph Hellwig wrote:
> > > > Fix this bug by saving the address of the physical location
> > > > of the ibft; later the driver will use isa_bus_to_virt() to get
> > > > the correct virtual address.
> > > 
> > > That sound rather broken.  Why not save the physical address in
> > > find_ibft_region and then later ioremap that when a virtual address is
> > > needed like all other code accessing magic I/O memory?
> > 
> > That is kind of what he does. The physical address is saved as a global
> > static variable and also the physical address is memreserved. Then
> > later on the physical address is used to create the virtual address.
> 
> Except that it uses isa_bus_to_virt, which is really broken.
> 
> > Or are you thinking of making the find_ibft_region reserve the physical
> > address, and _cache_ the physical address so there is no global
> > variable ?
> 
> No.  Just switch to ioremap/early_ioremap insted of this isa_bus_to_virt
> mess.

Why ioremap? This is not IO memory, plain phys_to_virt should be fine here.

-- 
Sincerely yours,
Mike.