All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad@darnok.org>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Greg KH <gregkh@suse.de>,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org,
	pjones@redhat.com
Subject: Re: [GIT PULL] ibft fix for 3.2-rc6
Date: Sat, 17 Dec 2011 09:42:06 -0500	[thread overview]
Message-ID: <20111217144206.GF14816@konrad-lan> (raw)
In-Reply-To: <CAE9FiQXsnc5FqbhPaZLsFbZa1zz44M8k-9yP-ow814oPEEDhjQ@mail.gmail.com>

On Fri, Dec 16, 2011 at 04:40:34PM -0800, Yinghai Lu wrote:
> On Thu, Dec 15, 2011 at 4:37 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Thu, Dec 15, 2011 at 4:07 PM, Greg KH <gregkh@suse.de> wrote:
> >
> >> The patch as-is does not apply to 2.6.32, so I can't apply it there.  If
> >> you want it applied there, please provide the backport to the above
> >> mentioned email address.
> 
> please check back ported patch for 2.6.36 to 2.6.39
> 
> We don't need to this one for 2.6.32, because EFI/ACPI ibft detecting
> is only added from 2.6.36.

Then 2.6.36 and above should have the fix below. Thanks for porting it
over.
> 
> but 2.6.32 base kernel still could take iscsi_ibft.c related changes
> in the patch, and ignore conflicts with iscs_ibft_find.c.
> but that will add new feature to 2.6.32. Not sure if stable tree
> should take that kind of change.

It could, but I am not too thrilled about it.
> 
> Thanks
> 
> Yinghai

> [PATCH -stable] ibft: Fix finding IBFT ACPI table on UEFI
> 
> commit 935a9fee51c945b8942be2d7b4bae069167b4886 upstream.
> 
> it is backporting for:
> 	kernel 2.6.36, 2.6.37, 2.6.38, 2.6.39
> 
> Found one system with UEFI/iBFT, kernel does not detect the iBFT during
> iscsi_ibft module loading.
> 
> Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
> 	- specifically in setup_arch() before ACPI is enabled.
> 
> Try to split acpi checking code out and call that later
> 
> At that time ACPI iBFT already get permanent mapped with ioremap.
> So isa_virt_to_bus() will get wrong phys from right virt address.
> We could just skip that phys address printing.
> 
> For legacy one, print the found address early.
> 
> -v2: update comments and description according to Konrad.
> -v3: fix problem about module use case that is found by Konrad.
> -v4: use acpi_get_table() instead of acpi_table_parse() to handle module use case that is found by Konrad again..
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/firmware/iscsi_ibft.c      |   42 +++++++++++++++++++++++++++++++++++--
>  drivers/firmware/iscsi_ibft_find.c |   22 +------------------
>  2 files changed, 42 insertions(+), 22 deletions(-)
> 
> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
> @@ -738,6 +738,37 @@ static void __exit ibft_exit(void)
>  	ibft_cleanup();
>  }
>  
> +#ifdef CONFIG_ACPI
> +static const struct {
> +	char *sign;
> +} ibft_signs[] = {
> +	/*
> +	 * One spec says "IBFT", the other says "iBFT". We have to check
> +	 * for both.
> +	 */
> +	{ ACPI_SIG_IBFT },
> +	{ "iBFT" },
> +};
> +
> +static void __init acpi_find_ibft_region(void)
> +{
> +	int i;
> +	struct acpi_table_header *table = NULL;
> +
> +	if (acpi_disabled)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++) {
> +		acpi_get_table(ibft_signs[i].sign, 0, &table);
> +		ibft_addr = (struct acpi_table_ibft *)table;
> +	}
> +}
> +#else
> +static void __init acpi_find_ibft_region(void)
> +{
> +}
> +#endif
> +
>  /*
>   * ibft_init() - creates sysfs tree entries for the iBFT data.
>   */
> @@ -745,9 +776,16 @@ static int __init ibft_init(void)
>  {
>  	int rc = 0;
>  
> +	/*
> +	   As on UEFI systems the setup_arch()/find_ibft_region()
> +	   is called before ACPI tables are parsed and it only does
> +	   legacy finding.
> +	*/
> +	if (!ibft_addr)
> +		acpi_find_ibft_region();
> +
>  	if (ibft_addr) {
> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
> -		       (u64)isa_virt_to_bus(ibft_addr));
> +		pr_info("iBFT detected.\n");
>  
>  		rc = ibft_check_device();
>  		if (rc)
> Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
> @@ -49,14 +49,6 @@ EXPORT_SYMBOL_GPL(ibft_addr);
>  #define VGA_MEM 0xA0000 /* VGA buffer */
>  #define VGA_SIZE 0x20000 /* 128kB */
>  
> -#ifdef CONFIG_ACPI
> -static int __init acpi_find_ibft(struct acpi_table_header *header)
> -{
> -	ibft_addr = (struct acpi_table_ibft *)header;
> -	return 0;
> -}
> -#endif /* CONFIG_ACPI */
> -
>  static int __init find_ibft_in_mem(void)
>  {
>  	unsigned long pos;
> @@ -77,6 +69,7 @@ static int __init find_ibft_in_mem(void)
>  			 * the table cannot be valid. */
>  			if (pos + len <= (IBFT_END-1)) {
>  				ibft_addr = (struct acpi_table_ibft *)virt;
> +				pr_info("iBFT found at 0x%lx.\n", pos);
>  				break;
>  			}
>  		}
> @@ -92,21 +85,10 @@ unsigned long __init find_ibft_region(un
>  
>  	ibft_addr = NULL;
>  
> -#ifdef CONFIG_ACPI
> -	/*
> -	 * One spec says "IBFT", the other says "iBFT". We have to check
> -	 * for both.
> -	 */
> -	if (!ibft_addr)
> -		acpi_table_parse(ACPI_SIG_IBFT, acpi_find_ibft);
> -	if (!ibft_addr)
> -		acpi_table_parse(IBFT_SIGN, acpi_find_ibft);
> -#endif /* CONFIG_ACPI */
> -
>  	/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
>  	 * only use ACPI for this */
>  
> -	if (!ibft_addr && !efi_enabled)
> +	if (!efi_enabled)
>  		find_ibft_in_mem();
>  
>  	if (ibft_addr) {


WARNING: multiple messages have this Message-ID (diff)
From: Konrad Rzeszutek Wilk <konrad@darnok.org>
To: Yinghai Lu <yinghai@kernel.org>
Cc: Greg KH <gregkh@suse.de>,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org,
	pjones@redhat.com
Subject: Re: [GIT PULL] ibft fix for 3.2-rc6
Date: Sat, 17 Dec 2011 09:42:06 -0500	[thread overview]
Message-ID: <20111217144206.GF14816@konrad-lan> (raw)
In-Reply-To: <CAE9FiQXsnc5FqbhPaZLsFbZa1zz44M8k-9yP-ow814oPEEDhjQ@mail.gmail.com>

On Fri, Dec 16, 2011 at 04:40:34PM -0800, Yinghai Lu wrote:
> On Thu, Dec 15, 2011 at 4:37 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Thu, Dec 15, 2011 at 4:07 PM, Greg KH <gregkh@suse.de> wrote:
> >
> >> The patch as-is does not apply to 2.6.32, so I can't apply it there. �If
> >> you want it applied there, please provide the backport to the above
> >> mentioned email address.
> 
> please check back ported patch for 2.6.36 to 2.6.39
> 
> We don't need to this one for 2.6.32, because EFI/ACPI ibft detecting
> is only added from 2.6.36.

Then 2.6.36 and above should have the fix below. Thanks for porting it
over.
> 
> but 2.6.32 base kernel still could take iscsi_ibft.c related changes
> in the patch, and ignore conflicts with iscs_ibft_find.c.
> but that will add new feature to 2.6.32. Not sure if stable tree
> should take that kind of change.

It could, but I am not too thrilled about it.
> 
> Thanks
> 
> Yinghai

> [PATCH -stable] ibft: Fix finding IBFT ACPI table on UEFI
> 
> commit 935a9fee51c945b8942be2d7b4bae069167b4886 upstream.
> 
> it is backporting for:
> 	kernel 2.6.36, 2.6.37, 2.6.38, 2.6.39
> 
> Found one system with UEFI/iBFT, kernel does not detect the iBFT during
> iscsi_ibft module loading.
> 
> Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
> 	- specifically in setup_arch() before ACPI is enabled.
> 
> Try to split acpi checking code out and call that later
> 
> At that time ACPI iBFT already get permanent mapped with ioremap.
> So isa_virt_to_bus() will get wrong phys from right virt address.
> We could just skip that phys address printing.
> 
> For legacy one, print the found address early.
> 
> -v2: update comments and description according to Konrad.
> -v3: fix problem about module use case that is found by Konrad.
> -v4: use acpi_get_table() instead of acpi_table_parse() to handle module use case that is found by Konrad again..
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/firmware/iscsi_ibft.c      |   42 +++++++++++++++++++++++++++++++++++--
>  drivers/firmware/iscsi_ibft_find.c |   22 +------------------
>  2 files changed, 42 insertions(+), 22 deletions(-)
> 
> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
> @@ -738,6 +738,37 @@ static void __exit ibft_exit(void)
>  	ibft_cleanup();
>  }
>  
> +#ifdef CONFIG_ACPI
> +static const struct {
> +	char *sign;
> +} ibft_signs[] = {
> +	/*
> +	 * One spec says "IBFT", the other says "iBFT". We have to check
> +	 * for both.
> +	 */
> +	{ ACPI_SIG_IBFT },
> +	{ "iBFT" },
> +};
> +
> +static void __init acpi_find_ibft_region(void)
> +{
> +	int i;
> +	struct acpi_table_header *table = NULL;
> +
> +	if (acpi_disabled)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++) {
> +		acpi_get_table(ibft_signs[i].sign, 0, &table);
> +		ibft_addr = (struct acpi_table_ibft *)table;
> +	}
> +}
> +#else
> +static void __init acpi_find_ibft_region(void)
> +{
> +}
> +#endif
> +
>  /*
>   * ibft_init() - creates sysfs tree entries for the iBFT data.
>   */
> @@ -745,9 +776,16 @@ static int __init ibft_init(void)
>  {
>  	int rc = 0;
>  
> +	/*
> +	   As on UEFI systems the setup_arch()/find_ibft_region()
> +	   is called before ACPI tables are parsed and it only does
> +	   legacy finding.
> +	*/
> +	if (!ibft_addr)
> +		acpi_find_ibft_region();
> +
>  	if (ibft_addr) {
> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
> -		       (u64)isa_virt_to_bus(ibft_addr));
> +		pr_info("iBFT detected.\n");
>  
>  		rc = ibft_check_device();
>  		if (rc)
> Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
> @@ -49,14 +49,6 @@ EXPORT_SYMBOL_GPL(ibft_addr);
>  #define VGA_MEM 0xA0000 /* VGA buffer */
>  #define VGA_SIZE 0x20000 /* 128kB */
>  
> -#ifdef CONFIG_ACPI
> -static int __init acpi_find_ibft(struct acpi_table_header *header)
> -{
> -	ibft_addr = (struct acpi_table_ibft *)header;
> -	return 0;
> -}
> -#endif /* CONFIG_ACPI */
> -
>  static int __init find_ibft_in_mem(void)
>  {
>  	unsigned long pos;
> @@ -77,6 +69,7 @@ static int __init find_ibft_in_mem(void)
>  			 * the table cannot be valid. */
>  			if (pos + len <= (IBFT_END-1)) {
>  				ibft_addr = (struct acpi_table_ibft *)virt;
> +				pr_info("iBFT found at 0x%lx.\n", pos);
>  				break;
>  			}
>  		}
> @@ -92,21 +85,10 @@ unsigned long __init find_ibft_region(un
>  
>  	ibft_addr = NULL;
>  
> -#ifdef CONFIG_ACPI
> -	/*
> -	 * One spec says "IBFT", the other says "iBFT". We have to check
> -	 * for both.
> -	 */
> -	if (!ibft_addr)
> -		acpi_table_parse(ACPI_SIG_IBFT, acpi_find_ibft);
> -	if (!ibft_addr)
> -		acpi_table_parse(IBFT_SIGN, acpi_find_ibft);
> -#endif /* CONFIG_ACPI */
> -
>  	/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
>  	 * only use ACPI for this */
>  
> -	if (!ibft_addr && !efi_enabled)
> +	if (!efi_enabled)
>  		find_ibft_in_mem();
>  
>  	if (ibft_addr) {


  reply	other threads:[~2011-12-17 14:42 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-15 21:05 [GIT PULL] ibft fix for 3.2-rc6 Konrad Rzeszutek Wilk
2011-12-15 23:15 ` Yinghai Lu
2011-12-16  0:07   ` Greg KH
2011-12-16  0:37     ` Yinghai Lu
2011-12-17  0:40       ` Yinghai Lu
2011-12-17 14:42         ` Konrad Rzeszutek Wilk [this message]
2011-12-17 14:42           ` Konrad Rzeszutek Wilk
2011-12-23  0:45           ` Yinghai Lu
2011-12-23  3:50             ` Konrad Rzeszutek Wilk
2011-12-23 18:24               ` Yinghai Lu
2011-12-22 20:58         ` Greg KH
2011-12-22 20:58           ` Greg KH

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=20111217144206.GF14816@konrad-lan \
    --to=konrad@darnok.org \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pjones@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=yinghai@kernel.org \
    /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.