| Message ID | 1331641907-8348-1-git-send-email-bos@je-eigen-domein.nl |
|---|---|
| State | New |
| Headers | show |
Il 13/03/2012 13:31, Floris Bos ha scritto: > Linux guests can address disks by their unique World Wide Name number (e.g. /dev/disk/by-id/wwn-0x5001517959123522) > This patch adds support for assigning a World Wide Name number to a virtual IDE disk. > > Cc: kwolf@redhat.com > Signed-off-by: Floris Bos <dev@noc-ps.com> > --- > hw/ide/core.c | 29 +++++++++++++++++++++++------ > hw/ide/internal.h | 5 ++++- > hw/ide/qdev.c | 3 ++- > 3 files changed, 29 insertions(+), 8 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 3e50c52..79b4f69 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -143,8 +143,12 @@ static void ide_identify(IDEState *s) > put_le16(p + 82, (1 << 14) | (1 << 5) | 1); > /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ > put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); Bit 14 should not be set here. Not your fault, but perhaps you can fix it too? > - /* 14=set to 1, 1=SMART self test, 0=SMART error logging */ > - put_le16(p + 84, (1 << 14) | 0); > + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ > + if (s->wwn) { > + put_le16(p + 84, (1 << 14) | (1 << 8) | 0); > + } else { > + put_le16(p + 84, (1 << 14) | 0); > + } > /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */ > if (bdrv_enable_write_cache(s->bs)) > put_le16(p + 85, (1 << 14) | (1 << 5) | 1); > @@ -152,8 +156,12 @@ static void ide_identify(IDEState *s) > put_le16(p + 85, (1 << 14) | 1); > /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ > put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); > - /* 14=set to 1, 1=smart self test, 0=smart error logging */ > - put_le16(p + 87, (1 << 14) | 0); > + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ > + if (s->wwn) { > + put_le16(p + 87, (1 << 14) | (1 << 8) | 0); > + } else { > + put_le16(p + 87, (1 << 14) | 0); > + } > put_le16(p + 88, 0x3f | (1 << 13)); /* udma5 set and supported */ > put_le16(p + 93, 1 | (1 << 14) | 0x2000); > put_le16(p + 100, s->nb_sectors); > @@ -163,6 +171,13 @@ static void ide_identify(IDEState *s) > > if (dev && dev->conf.physical_block_size) > put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf)); > + if (s->wwn) { > + /* LE 16-bit words 111-108 contain 64-bit World Wide Name */ > + put_le16(p + 108, s->wwn >> 48); > + put_le16(p + 109, s->wwn >> 32); > + put_le16(p + 110, s->wwn >> 16); > + put_le16(p + 111, s->wwn); > + } > if (dev && dev->conf.discard_granularity) { > put_le16(p + 169, 1); /* TRIM support */ > } Words 84, 87, 108-111 should also be set in the ide_atapi_identify routine: - 84 and 87 to 0x4000 if no WWN, 0x4100 if WWN provided - 108..111 to the WWN in the same awkward-endian layout. Otherwise looks good. Paolo > @@ -1834,7 +1849,8 @@ static const BlockDevOps ide_cd_block_ops = { > }; > > int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, > - const char *version, const char *serial, const char *model) > + const char *version, const char *serial, const char *model, > + uint64_t wwn) > { > int cylinders, heads, secs; > uint64_t nb_sectors; > @@ -1860,6 +1876,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, > s->heads = heads; > s->sectors = secs; > s->nb_sectors = nb_sectors; > + s->wwn = wwn; > /* The SMART values should be preserved across power cycles > but they aren't. */ > s->smart_enabled = 1; > @@ -1994,7 +2011,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, > if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, > dinfo->media_cd ? IDE_CD : IDE_HD, NULL, > *dinfo->serial ? dinfo->serial : NULL, > - NULL) < 0) { > + NULL, 0) < 0) { > error_report("Can't set up IDE drive %s", dinfo->id); > exit(1); > } > diff --git a/hw/ide/internal.h b/hw/ide/internal.h > index b1319dc..100efd3 100644 > --- a/hw/ide/internal.h > +++ b/hw/ide/internal.h > @@ -349,6 +349,7 @@ struct IDEState { > int drive_serial; > char drive_serial_str[21]; > char drive_model_str[41]; > + uint64_t wwn; > /* ide regs */ > uint8_t feature; > uint8_t error; > @@ -470,6 +471,7 @@ struct IDEDevice { > char *version; > char *serial; > char *model; > + uint64_t wwn; > }; > > #define BM_STATUS_DMAING 0x01 > @@ -536,7 +538,8 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); > uint32_t ide_data_readl(void *opaque, uint32_t addr); > > int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, > - const char *version, const char *serial, const char *model); > + const char *version, const char *serial, const char *model, > + uint64_t wwn); > void ide_init2(IDEBus *bus, qemu_irq irq); > void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, > DriveInfo *hd1, qemu_irq irq); > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c > index 07227c7..a46578d 100644 > --- a/hw/ide/qdev.c > +++ b/hw/ide/qdev.c > @@ -137,7 +137,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) > } > > if (ide_init_drive(s, dev->conf.bs, kind, > - dev->version, serial, dev->model) < 0) { > + dev->version, serial, dev->model, dev->wwn) < 0) { > return -1; > } > > @@ -174,6 +174,7 @@ static int ide_drive_initfn(IDEDevice *dev) > #define DEFINE_IDE_DEV_PROPERTIES() \ > DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \ > DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \ > + DEFINE_PROP_HEX64("wwn", IDEDrive, dev.wwn, 0), \ > DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\ > DEFINE_PROP_STRING("model", IDEDrive, dev.model) >
Am 13.03.2012 14:01, schrieb Paolo Bonzini: > Il 13/03/2012 13:31, Floris Bos ha scritto: >> Linux guests can address disks by their unique World Wide Name number (e.g. /dev/disk/by-id/wwn-0x5001517959123522) >> This patch adds support for assigning a World Wide Name number to a virtual IDE disk. >> >> Cc: kwolf@redhat.com >> Signed-off-by: Floris Bos <dev@noc-ps.com> >> --- >> hw/ide/core.c | 29 +++++++++++++++++++++++------ >> hw/ide/internal.h | 5 ++++- >> hw/ide/qdev.c | 3 ++- >> 3 files changed, 29 insertions(+), 8 deletions(-) >> >> diff --git a/hw/ide/core.c b/hw/ide/core.c >> index 3e50c52..79b4f69 100644 >> --- a/hw/ide/core.c >> +++ b/hw/ide/core.c >> @@ -143,8 +143,12 @@ static void ide_identify(IDEState *s) >> put_le16(p + 82, (1 << 14) | (1 << 5) | 1); >> /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ >> put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); > > Bit 14 should not be set here. Not your fault, but perhaps you can fix > it too? Table in the spec says: "14 - Shall be set to one" >> - /* 14=set to 1, 1=SMART self test, 0=SMART error logging */ >> - put_le16(p + 84, (1 << 14) | 0); >> + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ >> + if (s->wwn) { >> + put_le16(p + 84, (1 << 14) | (1 << 8) | 0); >> + } else { >> + put_le16(p + 84, (1 << 14) | 0); >> + } >> /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */ >> if (bdrv_enable_write_cache(s->bs)) >> put_le16(p + 85, (1 << 14) | (1 << 5) | 1); >> @@ -152,8 +156,12 @@ static void ide_identify(IDEState *s) >> put_le16(p + 85, (1 << 14) | 1); >> /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ >> put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); >> - /* 14=set to 1, 1=smart self test, 0=smart error logging */ >> - put_le16(p + 87, (1 << 14) | 0); >> + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ >> + if (s->wwn) { >> + put_le16(p + 87, (1 << 14) | (1 << 8) | 0); >> + } else { >> + put_le16(p + 87, (1 << 14) | 0); >> + } >> put_le16(p + 88, 0x3f | (1 << 13)); /* udma5 set and supported */ >> put_le16(p + 93, 1 | (1 << 14) | 0x2000); >> put_le16(p + 100, s->nb_sectors); >> @@ -163,6 +171,13 @@ static void ide_identify(IDEState *s) >> >> if (dev && dev->conf.physical_block_size) >> put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf)); >> + if (s->wwn) { >> + /* LE 16-bit words 111-108 contain 64-bit World Wide Name */ >> + put_le16(p + 108, s->wwn >> 48); >> + put_le16(p + 109, s->wwn >> 32); >> + put_le16(p + 110, s->wwn >> 16); >> + put_le16(p + 111, s->wwn); >> + } >> if (dev && dev->conf.discard_granularity) { >> put_le16(p + 169, 1); /* TRIM support */ >> } > > Words 84, 87, 108-111 should also be set in the ide_atapi_identify routine: > > - 84 and 87 to 0x4000 if no WWN, 0x4100 if WWN provided > > - 108..111 to the WWN in the same awkward-endian layout. What about ide_cfata_identify, while we're at it? Or isn't it supported there? But both of them are more missing features than bugs. I'll defer to Floris whether he wants to send another version. Kevin
Il 13/03/2012 14:22, Kevin Wolf ha scritto: >>> >> put_le16(p + 82, (1 << 14) | (1 << 5) | 1); >>> >> /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ >>> >> put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); >> > >> > Bit 14 should not be set here. Not your fault, but perhaps you can fix >> > it too? > > Table in the spec says: "14 - Shall be set to one" uff, I hate those pages. The wrong one is word 86. Bit 14 is reserved there. It's wrong in ide_cfata_identify, too. >>> >> - /* 14=set to 1, 1=SMART self test, 0=SMART error logging */ >>> >> - put_le16(p + 84, (1 << 14) | 0); >>> >> + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ >>> >> + if (s->wwn) { >>> >> + put_le16(p + 84, (1 << 14) | (1 << 8) | 0); >>> >> + } else { >>> >> + put_le16(p + 84, (1 << 14) | 0); >>> >> + } >>> >> /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */ >>> >> if (bdrv_enable_write_cache(s->bs)) >>> >> put_le16(p + 85, (1 << 14) | (1 << 5) | 1); >>> >> @@ -152,8 +156,12 @@ static void ide_identify(IDEState *s) >>> >> put_le16(p + 85, (1 << 14) | 1); >>> >> /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ >>> >> put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); > What about ide_cfata_identify, while we're at it? Or isn't it supported > there? No idea. Paolo
On 03/13/2012 02:01 PM, Paolo Bonzini wrote: > Words 84, 87, 108-111 should also be set in the ide_atapi_identify > routine: - 84 and 87 to 0x4000 if no WWN, 0x4100 if WWN provided - > 108..111 to the WWN in the same awkward-endian layout. Otherwise looks > good. Paolo I leave it to someone else to write a patch for the ATAPI/CF-ATA stuff. I'm not sure if there's a real use-case for WWN's for removable storage. And I am not so familiar with the ATA specification that I feel confident enough to make changes that also result in different behavior when no WWN is present. 84 and 87 are currently not set at all in ide_atapi_identify. Yours sincerely, Floris Bos
Il 13/03/2012 14:53, Floris Bos / Maxnet ha scritto: >> Words 84, 87, 108-111 should also be set in the ide_atapi_identify >> routine: - 84 and 87 to 0x4000 if no WWN, 0x4100 if WWN provided - >> 108..111 to the WWN in the same awkward-endian layout. Otherwise looks >> good. Paolo > > I leave it to someone else to write a patch for the ATAPI/CF-ATA stuff. > I'm not sure if there's a real use-case for WWN's for removable storage. > > And I am not so familiar with the ATA specification that I feel > confident enough to make changes that also result in different behavior > when no WWN is present. > 84 and 87 are currently not set at all in ide_atapi_identify. Fair enough, thanks! Paolo
Am 13.03.2012 14:53, schrieb Floris Bos / Maxnet: > On 03/13/2012 02:01 PM, Paolo Bonzini wrote: >> Words 84, 87, 108-111 should also be set in the ide_atapi_identify >> routine: - 84 and 87 to 0x4000 if no WWN, 0x4100 if WWN provided - >> 108..111 to the WWN in the same awkward-endian layout. Otherwise looks >> good. Paolo > > I leave it to someone else to write a patch for the ATAPI/CF-ATA stuff. > I'm not sure if there's a real use-case for WWN's for removable storage. Okay, fine with me. Applied your patches to the block branch now. Kevin
diff --git a/hw/ide/core.c b/hw/ide/core.c index 3e50c52..79b4f69 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -143,8 +143,12 @@ static void ide_identify(IDEState *s) put_le16(p + 82, (1 << 14) | (1 << 5) | 1); /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ put_le16(p + 83, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); - /* 14=set to 1, 1=SMART self test, 0=SMART error logging */ - put_le16(p + 84, (1 << 14) | 0); + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ + if (s->wwn) { + put_le16(p + 84, (1 << 14) | (1 << 8) | 0); + } else { + put_le16(p + 84, (1 << 14) | 0); + } /* 14 = NOP supported, 5=WCACHE enabled, 0=SMART feature set enabled */ if (bdrv_enable_write_cache(s->bs)) put_le16(p + 85, (1 << 14) | (1 << 5) | 1); @@ -152,8 +156,12 @@ static void ide_identify(IDEState *s) put_le16(p + 85, (1 << 14) | 1); /* 13=flush_cache_ext,12=flush_cache,10=lba48 */ put_le16(p + 86, (1 << 14) | (1 << 13) | (1 <<12) | (1 << 10)); - /* 14=set to 1, 1=smart self test, 0=smart error logging */ - put_le16(p + 87, (1 << 14) | 0); + /* 14=set to 1, 8=has WWN, 1=SMART self test, 0=SMART error logging */ + if (s->wwn) { + put_le16(p + 87, (1 << 14) | (1 << 8) | 0); + } else { + put_le16(p + 87, (1 << 14) | 0); + } put_le16(p + 88, 0x3f | (1 << 13)); /* udma5 set and supported */ put_le16(p + 93, 1 | (1 << 14) | 0x2000); put_le16(p + 100, s->nb_sectors); @@ -163,6 +171,13 @@ static void ide_identify(IDEState *s) if (dev && dev->conf.physical_block_size) put_le16(p + 106, 0x6000 | get_physical_block_exp(&dev->conf)); + if (s->wwn) { + /* LE 16-bit words 111-108 contain 64-bit World Wide Name */ + put_le16(p + 108, s->wwn >> 48); + put_le16(p + 109, s->wwn >> 32); + put_le16(p + 110, s->wwn >> 16); + put_le16(p + 111, s->wwn); + } if (dev && dev->conf.discard_granularity) { put_le16(p + 169, 1); /* TRIM support */ } @@ -1834,7 +1849,8 @@ static const BlockDevOps ide_cd_block_ops = { }; int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, - const char *version, const char *serial, const char *model) + const char *version, const char *serial, const char *model, + uint64_t wwn) { int cylinders, heads, secs; uint64_t nb_sectors; @@ -1860,6 +1876,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, s->heads = heads; s->sectors = secs; s->nb_sectors = nb_sectors; + s->wwn = wwn; /* The SMART values should be preserved across power cycles but they aren't. */ s->smart_enabled = 1; @@ -1994,7 +2011,7 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, if (ide_init_drive(&bus->ifs[i], dinfo->bdrv, dinfo->media_cd ? IDE_CD : IDE_HD, NULL, *dinfo->serial ? dinfo->serial : NULL, - NULL) < 0) { + NULL, 0) < 0) { error_report("Can't set up IDE drive %s", dinfo->id); exit(1); } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index b1319dc..100efd3 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -349,6 +349,7 @@ struct IDEState { int drive_serial; char drive_serial_str[21]; char drive_model_str[41]; + uint64_t wwn; /* ide regs */ uint8_t feature; uint8_t error; @@ -470,6 +471,7 @@ struct IDEDevice { char *version; char *serial; char *model; + uint64_t wwn; }; #define BM_STATUS_DMAING 0x01 @@ -536,7 +538,8 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val); uint32_t ide_data_readl(void *opaque, uint32_t addr); int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind, - const char *version, const char *serial, const char *model); + const char *version, const char *serial, const char *model, + uint64_t wwn); void ide_init2(IDEBus *bus, qemu_irq irq); void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1, qemu_irq irq); diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index 07227c7..a46578d 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -137,7 +137,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) } if (ide_init_drive(s, dev->conf.bs, kind, - dev->version, serial, dev->model) < 0) { + dev->version, serial, dev->model, dev->wwn) < 0) { return -1; } @@ -174,6 +174,7 @@ static int ide_drive_initfn(IDEDevice *dev) #define DEFINE_IDE_DEV_PROPERTIES() \ DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \ DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \ + DEFINE_PROP_HEX64("wwn", IDEDrive, dev.wwn, 0), \ DEFINE_PROP_STRING("serial", IDEDrive, dev.serial),\ DEFINE_PROP_STRING("model", IDEDrive, dev.model)
Linux guests can address disks by their unique World Wide Name number (e.g. /dev/disk/by-id/wwn-0x5001517959123522) This patch adds support for assigning a World Wide Name number to a virtual IDE disk. Cc: kwolf@redhat.com Signed-off-by: Floris Bos <dev@noc-ps.com> --- hw/ide/core.c | 29 +++++++++++++++++++++++------ hw/ide/internal.h | 5 ++++- hw/ide/qdev.c | 3 ++- 3 files changed, 29 insertions(+), 8 deletions(-)