All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>, Leo Yan <leo.yan@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <Pawel.Moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Bintian Wang <bintian.wang@huawei.com>,
	Yiping Xu <xuyiping@hisilicon.com>, Wei Xu <xuwei5@hisilicon.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"guodong.xu@linaro.org" <guodong.xu@linaro.org>,
	Jian Zhang <zhangjian001@hisilicon.com>,
	Zhenwei Wang <Zhenwei.wang@hisilicon.com>,
	Haoju Mo <mohaoju@hisilicon.com>,
	Dan Zhao <dan.zhao@hisilicon.com>,
	"kongfei@hisilicon.com" <kongfei@hisilicon.com>,
	Guangyue Zeng <zengguangyue@hisilicon.com>
Subject: Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
Date: Tue, 25 Aug 2015 11:40:53 +0100	[thread overview]
Message-ID: <20150825104053.GV10728@bivouac.eciton.net> (raw)
In-Reply-To: <1440497710.10987.42.camel@linaro.org>

On Tue, Aug 25, 2015 at 06:15:10PM +0800, Haojian Zhuang wrote:
> > > 1. We need support both UEFI and uboot. So the reserved buffer have to
> > > be declared in DTB since they are used by kernel driver, not UEFI.
> > 
> > The buffer may need to be declared in DTB also, but it most certanily
> > needs to be declared in UEFI.
> > 
> > And for the U-Boot case, since it is not memory available to Linux, it
> > should not be declared as "memory".
> 
> Something are messed at here. We have these buffer are used in mailbox.
> They should be allocated as non-cacheable.

That is a completely different issue, and if that is not currently
possible, then we need to fix that. But it needs to be fixed in the
right place.

> If these buffers are contained in memory memblock in kernel, it means
> that they exist in kernel page table with cachable property. When it's
> used in mailbox driver with non-cachable property, it'll only cause
> cache maintenance issue. So Leo declared these buffers as reserved
> in DT with "no-map" property. It's the key. It could avoid the cache
> maintenance issue.

Yes, when not booting with UEFI.

> > > 2. UEFI just loads grub. It's no time to run any other custom EFI
> > > application.
> > 
> > Apart from being completely irrelevant, how are you intending to
> > validate that GRUB never touches these memory regions?
> 
> GRUB is just a part of bootloader. When linux kernel is running,
> who cares GRUB? GRUB's lifetime is already finished.

We don't care once Linux is running - we care between UEFI boot
services starting and Linux memblock being initialised.

> By the way, UEFI code region is at [0x3Dxx_xxxx, 0x3DFF_FFFF]. Those
> mailbox buffer is in [0x05e0_xxxx, 0x06f0_xxxx]. Then I can make sure
> UEFI won't touch the reserved buffer.

And if a UEFI application explicitly requests to map an area
elsewhere, will your UEFI reject that request? How will it do that
without having information in its memory map about areas it must not
access?

> Even if UEFI touched the reserved
> buffer, is it an issue? Definitely it's not. UEFI's lifetime is end
> when linux kernel is running at hikey. Even if UEFI runtime service
> is enabled, the runtime data area is at [0x38xx_xxxx, 0x38xx_xxxx].

The runtime data area is currently, in your current image, at
[0x38xx_xxxx, 0x38xx_xxxx].

What happens if a UEFI application registers a configuration table?
Or registers a protocol for use at runtime?

Areas of memory that are not available for UEFI _must_ be marked as
such in the UEFI memory map. Once they are, we can deal with them in
the kernel. If this is not currently being done, that is a bug that
needs fixing.

> > Build a version once, test it, and hope the results remain valid
> > forever? And then when you move the regions and the previously working
> > GRUB now tramples all over them? Or when something changes in upstream
> > GRUB and its memory allocations drifts into the secretly untouchable
> > regions?
> 
> As I said above, UEFI won't touch it. And even UEFI touch it, kernel
> doesn't care since UEFI's lifetime is end.

UEFI's lifetime doesn't end until reset.

> > Are you then going to hack GRUB, release a special HiKey version of
> > GRUB, not support any other versions, and still can your firmware
> > UEFI?
> 
> I don't need to hack GRUB at all.

You will if you're running it under a "UEFI" which has areas you can't
touch and aren't telling it about that.

/
    Leif

WARNING: multiple messages have this Message-ID (diff)
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>, Leo Yan <leo.yan@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <Pawel.Moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Jassi Brar <jassisinghbrar@gmail.com>,
	Bintian Wang <bintian.wang@huawei.com>,
	Yiping Xu <xuyiping@hisilicon.com>, Wei Xu <xuwei5@hisilicon.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"guodong.xu@linaro.org" <guodong.xu@linaro.org>,
	Jian Zhang <zhangjian001@hisilicon.com>,
	Zhenwei Wang <Zhenwei.wang@hisilicon.com>,
	Haoju Mo <mohaoju@hisilicon.com>,
	Dan Zhao <dan.zhao@hisilicon.com>,
	"kongfei@hisilicon.com" <kongfei@>
Subject: Re: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
Date: Tue, 25 Aug 2015 11:40:53 +0100	[thread overview]
Message-ID: <20150825104053.GV10728@bivouac.eciton.net> (raw)
In-Reply-To: <1440497710.10987.42.camel@linaro.org>

On Tue, Aug 25, 2015 at 06:15:10PM +0800, Haojian Zhuang wrote:
> > > 1. We need support both UEFI and uboot. So the reserved buffer have to
> > > be declared in DTB since they are used by kernel driver, not UEFI.
> > 
> > The buffer may need to be declared in DTB also, but it most certanily
> > needs to be declared in UEFI.
> > 
> > And for the U-Boot case, since it is not memory available to Linux, it
> > should not be declared as "memory".
> 
> Something are messed at here. We have these buffer are used in mailbox.
> They should be allocated as non-cacheable.

That is a completely different issue, and if that is not currently
possible, then we need to fix that. But it needs to be fixed in the
right place.

> If these buffers are contained in memory memblock in kernel, it means
> that they exist in kernel page table with cachable property. When it's
> used in mailbox driver with non-cachable property, it'll only cause
> cache maintenance issue. So Leo declared these buffers as reserved
> in DT with "no-map" property. It's the key. It could avoid the cache
> maintenance issue.

Yes, when not booting with UEFI.

> > > 2. UEFI just loads grub. It's no time to run any other custom EFI
> > > application.
> > 
> > Apart from being completely irrelevant, how are you intending to
> > validate that GRUB never touches these memory regions?
> 
> GRUB is just a part of bootloader. When linux kernel is running,
> who cares GRUB? GRUB's lifetime is already finished.

We don't care once Linux is running - we care between UEFI boot
services starting and Linux memblock being initialised.

> By the way, UEFI code region is at [0x3Dxx_xxxx, 0x3DFF_FFFF]. Those
> mailbox buffer is in [0x05e0_xxxx, 0x06f0_xxxx]. Then I can make sure
> UEFI won't touch the reserved buffer.

And if a UEFI application explicitly requests to map an area
elsewhere, will your UEFI reject that request? How will it do that
without having information in its memory map about areas it must not
access?

> Even if UEFI touched the reserved
> buffer, is it an issue? Definitely it's not. UEFI's lifetime is end
> when linux kernel is running at hikey. Even if UEFI runtime service
> is enabled, the runtime data area is at [0x38xx_xxxx, 0x38xx_xxxx].

The runtime data area is currently, in your current image, at
[0x38xx_xxxx, 0x38xx_xxxx].

What happens if a UEFI application registers a configuration table?
Or registers a protocol for use at runtime?

Areas of memory that are not available for UEFI _must_ be marked as
such in the UEFI memory map. Once they are, we can deal with them in
the kernel. If this is not currently being done, that is a bug that
needs fixing.

> > Build a version once, test it, and hope the results remain valid
> > forever? And then when you move the regions and the previously working
> > GRUB now tramples all over them? Or when something changes in upstream
> > GRUB and its memory allocations drifts into the secretly untouchable
> > regions?
> 
> As I said above, UEFI won't touch it. And even UEFI touch it, kernel
> doesn't care since UEFI's lifetime is end.

UEFI's lifetime doesn't end until reset.

> > Are you then going to hack GRUB, release a special HiKey version of
> > GRUB, not support any other versions, and still can your firmware
> > UEFI?
> 
> I don't need to hack GRUB at all.

You will if you're running it under a "UEFI" which has areas you can't
touch and aren't telling it about that.

/
    Leif

WARNING: multiple messages have this Message-ID (diff)
From: leif.lindholm@linaro.org (Leif Lindholm)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node
Date: Tue, 25 Aug 2015 11:40:53 +0100	[thread overview]
Message-ID: <20150825104053.GV10728@bivouac.eciton.net> (raw)
In-Reply-To: <1440497710.10987.42.camel@linaro.org>

On Tue, Aug 25, 2015 at 06:15:10PM +0800, Haojian Zhuang wrote:
> > > 1. We need support both UEFI and uboot. So the reserved buffer have to
> > > be declared in DTB since they are used by kernel driver, not UEFI.
> > 
> > The buffer may need to be declared in DTB also, but it most certanily
> > needs to be declared in UEFI.
> > 
> > And for the U-Boot case, since it is not memory available to Linux, it
> > should not be declared as "memory".
> 
> Something are messed at here. We have these buffer are used in mailbox.
> They should be allocated as non-cacheable.

That is a completely different issue, and if that is not currently
possible, then we need to fix that. But it needs to be fixed in the
right place.

> If these buffers are contained in memory memblock in kernel, it means
> that they exist in kernel page table with cachable property. When it's
> used in mailbox driver with non-cachable property, it'll only cause
> cache maintenance issue. So Leo declared these buffers as reserved
> in DT with "no-map" property. It's the key. It could avoid the cache
> maintenance issue.

Yes, when not booting with UEFI.

> > > 2. UEFI just loads grub. It's no time to run any other custom EFI
> > > application.
> > 
> > Apart from being completely irrelevant, how are you intending to
> > validate that GRUB never touches these memory regions?
> 
> GRUB is just a part of bootloader. When linux kernel is running,
> who cares GRUB? GRUB's lifetime is already finished.

We don't care once Linux is running - we care between UEFI boot
services starting and Linux memblock being initialised.

> By the way, UEFI code region is at [0x3Dxx_xxxx, 0x3DFF_FFFF]. Those
> mailbox buffer is in [0x05e0_xxxx, 0x06f0_xxxx]. Then I can make sure
> UEFI won't touch the reserved buffer.

And if a UEFI application explicitly requests to map an area
elsewhere, will your UEFI reject that request? How will it do that
without having information in its memory map about areas it must not
access?

> Even if UEFI touched the reserved
> buffer, is it an issue? Definitely it's not. UEFI's lifetime is end
> when linux kernel is running at hikey. Even if UEFI runtime service
> is enabled, the runtime data area is at [0x38xx_xxxx, 0x38xx_xxxx].

The runtime data area is currently, in your current image, at
[0x38xx_xxxx, 0x38xx_xxxx].

What happens if a UEFI application registers a configuration table?
Or registers a protocol for use at runtime?

Areas of memory that are not available for UEFI _must_ be marked as
such in the UEFI memory map. Once they are, we can deal with them in
the kernel. If this is not currently being done, that is a bug that
needs fixing.

> > Build a version once, test it, and hope the results remain valid
> > forever? And then when you move the regions and the previously working
> > GRUB now tramples all over them? Or when something changes in upstream
> > GRUB and its memory allocations drifts into the secretly untouchable
> > regions?
> 
> As I said above, UEFI won't touch it. And even UEFI touch it, kernel
> doesn't care since UEFI's lifetime is end.

UEFI's lifetime doesn't end until reset.

> > Are you then going to hack GRUB, release a special HiKey version of
> > GRUB, not support any other versions, and still can your firmware
> > UEFI?
> 
> I don't need to hack GRUB at all.

You will if you're running it under a "UEFI" which has areas you can't
touch and aren't telling it about that.

/
    Leif

  reply	other threads:[~2015-08-25 10:40 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-19  9:37 [PATCH v1 0/3] mailbox: hisilicon: add Hi6220 mailbox driver Leo Yan
2015-08-19  9:37 ` Leo Yan
2015-08-19  9:37 ` Leo Yan
2015-08-19  9:37 ` [PATCH v1 1/3] dt-bindings: mailbox: Document " Leo Yan
2015-08-19  9:37   ` Leo Yan
2015-08-25 11:17   ` Sudeep Holla
2015-08-25 11:17     ` Sudeep Holla
2015-08-25 11:17     ` Sudeep Holla
2015-08-25 13:01     ` Leo Yan
2015-08-25 13:01       ` Leo Yan
2015-08-25 13:01       ` Leo Yan
2015-08-19  9:37 ` [PATCH v1 2/3] mailbox: Hi6220: add " Leo Yan
2015-08-19  9:37   ` Leo Yan
2015-08-19  9:37 ` [PATCH v1 3/3] arm64: dts: add Hi6220 mailbox node Leo Yan
2015-08-19  9:37   ` Leo Yan
2015-08-21 18:40   ` Mark Rutland
2015-08-21 18:40     ` Mark Rutland
2015-08-21 18:40     ` Mark Rutland
2015-08-22 13:30     ` Leo Yan
2015-08-22 13:30       ` Leo Yan
2015-08-22 13:30       ` Leo Yan
2015-08-24  3:27       ` Leo Yan
2015-08-24  3:27         ` Leo Yan
2015-08-24  3:27         ` Leo Yan
2015-08-24  9:18     ` Leo Yan
2015-08-24  9:18       ` Leo Yan
2015-08-24  9:18       ` Leo Yan
2015-08-24  9:51       ` Mark Rutland
2015-08-24  9:51         ` Mark Rutland
2015-08-24  9:51         ` Mark Rutland
2015-08-24 10:19         ` Haojian Zhuang
2015-08-24 10:19           ` Haojian Zhuang
2015-08-24 10:19           ` Haojian Zhuang
2015-08-24 11:49           ` Leif Lindholm
2015-08-24 11:49             ` Leif Lindholm
2015-08-24 11:49             ` Leif Lindholm
2015-08-25  8:13             ` Haojian Zhuang
2015-08-25  8:13               ` Haojian Zhuang
2015-08-25  8:13               ` Haojian Zhuang
2015-08-25  9:46               ` Leif Lindholm
2015-08-25  9:46                 ` Leif Lindholm
2015-08-25  9:46                 ` Leif Lindholm
2015-08-25 10:15                 ` Haojian Zhuang
2015-08-25 10:15                   ` Haojian Zhuang
2015-08-25 10:15                   ` Haojian Zhuang
2015-08-25 10:40                   ` Leif Lindholm [this message]
2015-08-25 10:40                     ` Leif Lindholm
2015-08-25 10:40                     ` Leif Lindholm
2015-08-25 10:42                   ` Mark Rutland
2015-08-25 10:42                     ` Mark Rutland
2015-08-25 10:42                     ` Mark Rutland
2015-08-25 13:43                     ` Haojian Zhuang
2015-08-25 13:43                       ` Haojian Zhuang
2015-08-25 13:43                       ` Haojian Zhuang
2015-08-25 14:24                       ` Leif Lindholm
2015-08-25 14:24                         ` Leif Lindholm
2015-08-25 14:24                         ` Leif Lindholm
2015-08-25 14:51                         ` Ard Biesheuvel
2015-08-25 14:51                           ` Ard Biesheuvel
2015-08-25 14:51                           ` Ard Biesheuvel
2015-08-25 15:37                           ` Leif Lindholm
2015-08-25 15:37                             ` Leif Lindholm
2015-08-25 15:37                             ` Leif Lindholm
2015-08-25 15:45                             ` Ard Biesheuvel
2015-08-25 15:45                               ` Ard Biesheuvel
2015-08-25 15:45                               ` Ard Biesheuvel
2015-08-26  2:41                             ` Haojian Zhuang
2015-08-26  2:41                               ` Haojian Zhuang
2015-08-26  2:41                               ` Haojian Zhuang
2015-08-25 16:00                       ` Leo Yan
2015-08-25 16:00                         ` Leo Yan
2015-08-25 16:00                         ` Leo Yan
2015-08-26  1:25                         ` Haojian Zhuang
2015-08-26  1:25                           ` Haojian Zhuang
2015-08-26  1:25                           ` Haojian Zhuang
2015-08-26  6:59                           ` Leo Yan
2015-08-26  6:59                             ` Leo Yan
2015-08-26  6:59                             ` Leo Yan
2015-08-27 16:31                             ` Mark Rutland
2015-08-27 16:31                               ` Mark Rutland
2015-08-27 16:31                               ` Mark Rutland
2015-08-28  6:37                               ` Leo Yan
2015-08-28  6:37                                 ` Leo Yan
2015-08-28  6:37                                 ` Leo Yan
2015-08-27 15:54                           ` Daniel Thompson
2015-08-27 15:54                             ` Daniel Thompson
2015-08-27 15:54                             ` Daniel Thompson
2015-08-27 16:46                           ` Mark Rutland
2015-08-27 16:46                             ` Mark Rutland
2015-08-27 16:46                             ` Mark Rutland
2015-08-24 12:48           ` Mark Rutland
2015-08-24 12:48             ` Mark Rutland
2015-08-24 12:48             ` Mark Rutland
2015-08-25  8:04             ` Haojian Zhuang
2015-08-25  8:04               ` Haojian Zhuang
2015-08-25  8:04               ` Haojian Zhuang
2015-08-25 11:09               ` Mark Rutland
2015-08-25 11:09                 ` Mark Rutland
2015-08-25 11:09                 ` Mark Rutland
2015-08-25 11:36   ` Sudeep Holla
2015-08-25 11:36     ` Sudeep Holla
2015-08-25 11:36     ` Sudeep Holla
2015-08-25 14:04     ` Leo Yan
2015-08-25 14:04       ` Leo Yan
2015-08-25 14:04       ` Leo Yan
2015-08-25 14:13       ` Sudeep Holla
2015-08-25 14:13         ` Sudeep Holla
2015-08-25 14:13         ` Sudeep Holla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150825104053.GV10728@bivouac.eciton.net \
    --to=leif.lindholm@linaro.org \
    --cc=Catalin.Marinas@arm.com \
    --cc=Pawel.Moll@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=Zhenwei.wang@hisilicon.com \
    --cc=bintian.wang@huawei.com \
    --cc=dan.zhao@hisilicon.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=guodong.xu@linaro.org \
    --cc=haojian.zhuang@linaro.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=jassisinghbrar@gmail.com \
    --cc=kongfei@hisilicon.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mohaoju@hisilicon.com \
    --cc=robh+dt@kernel.org \
    --cc=xuwei5@hisilicon.com \
    --cc=xuyiping@hisilicon.com \
    --cc=zengguangyue@hisilicon.com \
    --cc=zhangjian001@hisilicon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.