LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Konrad Rzeszutek <konrad@darnok.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Greg KH <greg@kroah.com>,
	michaelc@cs.wisc.edu, dwm@enoyolf.org, darnok@68k.org,
	pjones@redhat.com, konradr@redhat.com,
	konradr@linux.vnet.ibm.com, randy.dunlap@oracle.com,
	hpa@zytor.com, lenb@kernel.org, mike.anderson@us.ibm.com,
	dwm@austin.ibm.com, arjan@infradead.org,
	Andy Whitcroft <apw@shadowen.org>
Subject: Re: [PATCH] Add iSCSI iBFT support (v0.4.6)
Date: Fri, 01 Feb 2008 18:18:09 -0600	[thread overview]
Message-ID: <1201911489.3134.79.camel@localhost.localdomain> (raw)
In-Reply-To: <20080130213700.GA28529@andromeda.dapyr.net>

On Wed, 2008-01-30 at 17:37 -0400, Konrad Rzeszutek wrote:
> This patch (v0.4.6) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> directories along with text properties which export the the iSCSI
> Boot Firmware Table (iBFT) structure.
> 
> What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> tools to extract from the machine NICs the iSCSI connection information
> so that they can automagically mount the iSCSI share/target. Currently
> the iSCSI information is hard-coded in the initrd. The /sysfs entries
> are read-only one-name-and-value fields.
> 
> The usual set of data exposed is:
> 
> # for a in `find /sys/firmware/ibft/ -type f -print`; do  echo -n "$a: ";  cat $a; done
> /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb
> /sys/firmware/ibft/target0/nic-assoc: 0
> /sys/firmware/ibft/target0/chap-type: 0
> /sys/firmware/ibft/target0/lun: 00000000
> /sys/firmware/ibft/target0/port: 3260
> /sys/firmware/ibft/target0/ip-addr: 192.168.79.116
> /sys/firmware/ibft/target0/flags: 3
> /sys/firmware/ibft/target0/index: 0
> /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01
> /sys/firmware/ibft/ethernet0/vlan: 0
> /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254
> /sys/firmware/ibft/ethernet0/origin: 0
> /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0
> /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41
> /sys/firmware/ibft/ethernet0/flags: 7
> /sys/firmware/ibft/ethernet0/index: 0
> /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator
> /sys/firmware/ibft/initiator/flags: 3
> /sys/firmware/ibft/initiator/index: 0
> 
> 
> For full details of the IBFT structure please take a look at:
> ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf

Some pieces of the patch are obviously wrong:  find_ibft() shouldn't be
in ibft_init ... if ibft_phys was zero, it means the bootmem reservation
wasn't done and you shouldn't be poking about in memory which has likely
now been overwritten.

Also, why is ibft_phys the global variable you pass?  You never actually
want to use the physical address, what you always end up using is the
kernel virtual address.

I'd simply use the ibft variable to point to the virtual address of the
ibft or null if not found, then you can throw out all the phys_to_virt()
calls.

Also, move the reserve_bootmem into the ibft_find routines and ensure
they're only called once on boot.  Refuse to attach the ibft driver if
the virtual pointer ibft is null.

James



  reply	other threads:[~2008-02-02  0:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-30 21:37 Konrad Rzeszutek
2008-02-02  0:18 ` James Bottomley [this message]
2008-02-02  0:44   ` Andrew Morton
2008-02-08 20:37   ` Konrad Rzeszutek

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=1201911489.3134.79.camel@localhost.localdomain \
    --to=james.bottomley@hansenpartnership.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@shadowen.org \
    --cc=arjan@infradead.org \
    --cc=darnok@68k.org \
    --cc=dwm@austin.ibm.com \
    --cc=dwm@enoyolf.org \
    --cc=greg@kroah.com \
    --cc=hpa@zytor.com \
    --cc=konrad@darnok.org \
    --cc=konradr@linux.vnet.ibm.com \
    --cc=konradr@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=mike.anderson@us.ibm.com \
    --cc=pjones@redhat.com \
    --cc=randy.dunlap@oracle.com \
    --subject='Re: [PATCH] Add iSCSI iBFT support (v0.4.6)' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).