* [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
@ 2007-07-04 13:22 Geert Uytterhoeven
2007-07-04 13:22 ` [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver Geert Uytterhoeven
` (7 more replies)
0 siblings, 8 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]
Hi,
This is the fourth submission of the new PS3 storage drivers:
[1] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
[2] ps3: Storage Driver Core
[3] ps3: Storage device registration routines.
[4] ps3: Disk Storage Driver
[5] ps3: BD/DVD/CD-ROM Storage Driver
[6] ps3: FLASH ROM Storage Driver
They are based on:
- the current Linux kernel source tree,
- plus the PS3 patches already submitted by Geoff Levand on linuxppc-dev,
- plus the shost_priv() patch in scsi-misc (commit
bcd92c9fbcc679ee95003083056f0441a1f474fa).
All issues raised during the previous review rounds should be fixed.
There were no more comments after the third submission.
Paul already integrated Geoff Levand's PS3 patches and the first 3 patches
in this series in the for-2.6.23 branch in powerpc.git.
Maintainers of block/SCSI/misc, can you please ack the last 3 patches so Paul
can take them, too?
Thanks in advance!
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-04 13:22 ` [patch 2/6] ps3: Storage Driver Core Geert Uytterhoeven
` (6 subsequent siblings)
7 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geert Uytterhoeven, Geoff Levand
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_flash_bootmem.diff --]
[-- Type: text/plain, Size: 3163 bytes --]
Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage driver.
This can be disabled by passing `ps3flash=off' on the kernel command line.
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
v3:
- Allow to disable ps3flash (and the preallocated 256 KiB buffer) using
`ps3flash=off'
arch/powerpc/platforms/ps3/setup.c | 31 ++++++++++++++++++++++++++++++-
include/asm-powerpc/ps3.h | 1 +
2 files changed, 31 insertions(+), 1 deletion(-)
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -107,7 +107,8 @@ static void ps3_panic(char *str)
while(1);
}
-#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
+#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE) || \
+ defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
static void prealloc(struct ps3_prealloc *p)
{
if (!p->size)
@@ -123,7 +124,9 @@ static void prealloc(struct ps3_prealloc
printk(KERN_INFO "%s: %lu bytes at %p\n", p->name, p->size,
p->address);
}
+#endif
+#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
struct ps3_prealloc ps3fb_videomemory = {
.name = "ps3fb videomemory",
.size = CONFIG_FB_PS3_DEFAULT_SIZE_M*1024*1024,
@@ -146,6 +149,30 @@ early_param("ps3fb", early_parse_ps3fb);
#define prealloc_ps3fb_videomemory() do { } while (0)
#endif
+#if defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
+struct ps3_prealloc ps3flash_bounce_buffer = {
+ .name = "ps3flash bounce buffer",
+ .size = 256*1024,
+ .align = 256*1024
+};
+EXPORT_SYMBOL_GPL(ps3flash_bounce_buffer);
+#define prealloc_ps3flash_bounce_buffer() prealloc(&ps3flash_bounce_buffer)
+
+static int __init early_parse_ps3flash(char *p)
+{
+ if (!p)
+ return 1;
+
+ if (!strcmp(p, "off"))
+ ps3flash_bounce_buffer.size = 0;
+
+ return 0;
+}
+early_param("ps3flash", early_parse_ps3flash);
+#else
+#define prealloc_ps3flash_bounce_buffer() do { } while (0)
+#endif
+
static int ps3_set_dabr(u64 dabr)
{
enum {DABR_USER = 1, DABR_KERNEL = 2,};
@@ -175,6 +202,8 @@ static void __init ps3_setup_arch(void)
#endif
prealloc_ps3fb_videomemory();
+ prealloc_ps3flash_bounce_buffer();
+
ppc_md.power_save = ps3_power_save;
DBG(" <- %s:%d\n", __func__, __LINE__);
--- a/include/asm-powerpc/ps3.h
+++ b/include/asm-powerpc/ps3.h
@@ -427,6 +427,7 @@ struct ps3_prealloc {
};
extern struct ps3_prealloc ps3fb_videomemory;
+extern struct ps3_prealloc ps3flash_bounce_buffer;
#endif
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 2/6] ps3: Storage Driver Core
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-04 13:22 ` [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-04 13:22 ` [patch 3/6] ps3: Storage device registration routines Geert Uytterhoeven
` (5 subsequent siblings)
7 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geert Uytterhoeven, Geoff Levand
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_bus.diff --]
[-- Type: text/plain, Size: 13049 bytes --]
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Add storage driver core support for the PS3.
PS3 storage devices are a special kind of PS3 system bus devices
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
v3:
- Rename and move ps3stor_interrupt() to ps3flash_interrupt()
- Add an interrupt handler parameter to ps3stor_setup(), so it doesn't have
to be overridden later
v2:
- Remove the `name' parameters of ps3stor_setup(), as it can be obtained from
the driver structure
- Cleanup struct ps3_storage_device
arch/powerpc/platforms/ps3/Kconfig | 4
drivers/ps3/Makefile | 1
drivers/ps3/ps3stor_lib.c | 302 +++++++++++++++++++++++++++++++++++++
include/asm-powerpc/ps3stor.h | 71 ++++++++
4 files changed, 378 insertions(+)
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -98,4 +98,8 @@ config PS3_SYS_MANAGER
This support is required for system control. In
general, all users will say Y or M.
+config PS3_STORAGE
+ depends on PPC_PS3
+ tristate
+
endmenu
--- a/drivers/ps3/Makefile
+++ b/drivers/ps3/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_PS3_PS3AV) += ps3av_mod.o
ps3av_mod-objs += ps3av.o ps3av_cmd.o
obj-$(CONFIG_PPC_PS3) += sys-manager-core.o
obj-$(CONFIG_PS3_SYS_MANAGER) += sys-manager.o
+obj-$(CONFIG_PS3_STORAGE) += ps3stor_lib.o
--- /dev/null
+++ b/drivers/ps3/ps3stor_lib.c
@@ -0,0 +1,302 @@
+/*
+ * PS3 Storage Library
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/dma-mapping.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+
+
+static int ps3stor_probe_access(struct ps3_storage_device *dev)
+{
+ int res, error;
+ unsigned int i;
+ unsigned long n;
+
+ if (dev->sbd.match_id == PS3_MATCH_ID_STOR_ROM) {
+ /* special case: CD-ROM is assumed always accessible */
+ dev->accessible_regions = 1;
+ return 0;
+ }
+
+ error = -EPERM;
+ for (i = 0; i < dev->num_regions; i++) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: checking accessibility of region %u\n",
+ __func__, __LINE__, i);
+
+ dev->region_idx = i;
+ res = ps3stor_read_write_sectors(dev, dev->bounce_lpar, 0, 1,
+ 0);
+ if (res) {
+ dev_dbg(&dev->sbd.core, "%s:%u: read failed, "
+ "region %u is not accessible\n", __func__,
+ __LINE__, i);
+ continue;
+ }
+
+ dev_dbg(&dev->sbd.core, "%s:%u: region %u is accessible\n",
+ __func__, __LINE__, i);
+ set_bit(i, &dev->accessible_regions);
+
+ /* We can access at least one region */
+ error = 0;
+ }
+ if (error)
+ return error;
+
+ n = hweight_long(dev->accessible_regions);
+ if (n > 1)
+ dev_info(&dev->sbd.core,
+ "%s:%u: %lu accessible regions found. Only the first "
+ "one will be used",
+ __func__, __LINE__, n);
+ dev->region_idx = __ffs(dev->accessible_regions);
+ dev_info(&dev->sbd.core,
+ "First accessible region has index %u start %lu size %lu\n",
+ dev->region_idx, dev->regions[dev->region_idx].start,
+ dev->regions[dev->region_idx].size);
+
+ return 0;
+}
+
+
+/**
+ * ps3stor_setup - Setup a storage device before use
+ * @dev: Pointer to a struct ps3_storage_device
+ * @handler: Pointer to an interrupt handler
+ *
+ * Returns 0 for success, or an error code
+ */
+int ps3stor_setup(struct ps3_storage_device *dev, irq_handler_t handler)
+{
+ int error, res, alignment;
+ enum ps3_dma_page_size page_size;
+
+ error = ps3_open_hv_device(&dev->sbd);
+ if (error) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: ps3_open_hv_device failed %d\n", __func__,
+ __LINE__, error);
+ goto fail;
+ }
+
+ error = ps3_sb_event_receive_port_setup(&dev->sbd, PS3_BINDING_CPU_ANY,
+ &dev->irq);
+ if (error) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
+ __func__, __LINE__, error);
+ goto fail_close_device;
+ }
+
+ error = request_irq(dev->irq, handler, IRQF_DISABLED,
+ dev->sbd.core.driver->name, dev);
+ if (error) {
+ dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
+ __func__, __LINE__, error);
+ goto fail_sb_event_receive_port_destroy;
+ }
+
+ alignment = min(__ffs(dev->bounce_size),
+ __ffs((unsigned long)dev->bounce_buf));
+ if (alignment < 12) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: bounce buffer not aligned (%lx at 0x%p)\n",
+ __func__, __LINE__, dev->bounce_size, dev->bounce_buf);
+ error = -EINVAL;
+ goto fail_free_irq;
+ } else if (alignment < 16)
+ page_size = PS3_DMA_4K;
+ else
+ page_size = PS3_DMA_64K;
+ dev->sbd.d_region = &dev->dma_region;
+ ps3_dma_region_init(&dev->sbd, &dev->dma_region, page_size,
+ PS3_DMA_OTHER, dev->bounce_buf, dev->bounce_size);
+ res = ps3_dma_region_create(&dev->dma_region);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: cannot create DMA region\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_free_irq;
+ }
+
+ dev->bounce_lpar = ps3_mm_phys_to_lpar(__pa(dev->bounce_buf));
+ dev->bounce_dma = dma_map_single(&dev->sbd.core, dev->bounce_buf,
+ dev->bounce_size, DMA_BIDIRECTIONAL);
+ if (!dev->bounce_dma) {
+ dev_err(&dev->sbd.core, "%s:%u: map DMA region failed\n",
+ __func__, __LINE__);
+ error = -ENODEV;
+ goto fail_free_dma;
+ }
+
+ error = ps3stor_probe_access(dev);
+ if (error) {
+ dev_err(&dev->sbd.core, "%s:%u: No accessible regions found\n",
+ __func__, __LINE__);
+ goto fail_unmap_dma;
+ }
+ return 0;
+
+fail_unmap_dma:
+ dma_unmap_single(&dev->sbd.core, dev->bounce_dma, dev->bounce_size,
+ DMA_BIDIRECTIONAL);
+fail_free_dma:
+ ps3_dma_region_free(&dev->dma_region);
+fail_free_irq:
+ free_irq(dev->irq, dev);
+fail_sb_event_receive_port_destroy:
+ ps3_sb_event_receive_port_destroy(&dev->sbd, dev->irq);
+fail_close_device:
+ ps3_close_hv_device(&dev->sbd);
+fail:
+ return error;
+}
+EXPORT_SYMBOL_GPL(ps3stor_setup);
+
+
+/**
+ * ps3stor_teardown - Tear down a storage device after use
+ * @dev: Pointer to a struct ps3_storage_device
+ */
+void ps3stor_teardown(struct ps3_storage_device *dev)
+{
+ int error;
+
+ dma_unmap_single(&dev->sbd.core, dev->bounce_dma, dev->bounce_size,
+ DMA_BIDIRECTIONAL);
+ ps3_dma_region_free(&dev->dma_region);
+
+ free_irq(dev->irq, dev);
+
+ error = ps3_sb_event_receive_port_destroy(&dev->sbd, dev->irq);
+ if (error)
+ dev_err(&dev->sbd.core,
+ "%s:%u: destroy event receive port failed %d\n",
+ __func__, __LINE__, error);
+
+ error = ps3_close_hv_device(&dev->sbd);
+ if (error)
+ dev_err(&dev->sbd.core,
+ "%s:%u: ps3_close_hv_device failed %d\n", __func__,
+ __LINE__, error);
+}
+EXPORT_SYMBOL_GPL(ps3stor_teardown);
+
+
+/**
+ * ps3stor_read_write_sectors - read/write from/to a storage device
+ * @dev: Pointer to a struct ps3_storage_device
+ * @lpar: HV logical partition address
+ * @start_sector: First sector to read/write
+ * @sectors: Number of sectors to read/write
+ * @write: Flag indicating write (non-zero) or read (zero)
+ *
+ * Returns 0 for success, -1 in case of failure to submit the command, or
+ * an LV1 status value in case of other errors
+ */
+u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+ u64 start_sector, u64 sectors, int write)
+{
+ unsigned int region_id = dev->regions[dev->region_idx].id;
+ const char *op = write ? "write" : "read";
+ int res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: %s %lu sectors starting at %lu\n",
+ __func__, __LINE__, op, sectors, start_sector);
+
+ init_completion(&dev->done);
+ res = write ? lv1_storage_write(dev->sbd.dev_id, region_id,
+ start_sector, sectors, 0, lpar,
+ &dev->tag)
+ : lv1_storage_read(dev->sbd.dev_id, region_id,
+ start_sector, sectors, 0, lpar,
+ &dev->tag);
+ if (res) {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+ __LINE__, op, res);
+ return -1;
+ }
+
+ wait_for_completion(&dev->done);
+ if (dev->lv1_status) {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+ __LINE__, op, dev->lv1_status);
+ return dev->lv1_status;
+ }
+
+ dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__, __LINE__,
+ op);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ps3stor_read_write_sectors);
+
+
+/**
+ * ps3stor_send_command - send a device command to a storage device
+ * @dev: Pointer to a struct ps3_storage_device
+ * @cmd: Command number
+ * @arg1: First command argument
+ * @arg2: Second command argument
+ * @arg3: Third command argument
+ * @arg4: Fourth command argument
+ *
+ * Returns 0 for success, -1 in case of failure to submit the command, or
+ * an LV1 status value in case of other errors
+ */
+u64 ps3stor_send_command(struct ps3_storage_device *dev, u64 cmd, u64 arg1,
+ u64 arg2, u64 arg3, u64 arg4)
+{
+ int res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: send device command 0x%lx\n", __func__,
+ __LINE__, cmd);
+
+ init_completion(&dev->done);
+
+ res = lv1_storage_send_device_command(dev->sbd.dev_id, cmd, arg1,
+ arg2, arg3, arg4, &dev->tag);
+ if (res) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: send_device_command 0x%lx failed %d\n",
+ __func__, __LINE__, cmd, res);
+ return -1;
+ }
+
+ wait_for_completion(&dev->done);
+ if (dev->lv1_status) {
+ dev_dbg(&dev->sbd.core, "%s:%u: command 0x%lx failed 0x%lx\n",
+ __func__, __LINE__, cmd, dev->lv1_status);
+ return dev->lv1_status;
+ }
+
+ dev_dbg(&dev->sbd.core, "%s:%u: command 0x%lx completed\n", __func__,
+ __LINE__, cmd);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ps3stor_send_command);
+
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 Storage Bus Library");
+MODULE_AUTHOR("Sony Corporation");
--- /dev/null
+++ b/include/asm-powerpc/ps3stor.h
@@ -0,0 +1,71 @@
+/*
+ * PS3 Storage Devices
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef _ASM_POWERPC_PS3STOR_H_
+#define _ASM_POWERPC_PS3STOR_H_
+
+#include <linux/interrupt.h>
+
+#include <asm/ps3.h>
+
+
+struct ps3_storage_region {
+ unsigned int id;
+ u64 start;
+ u64 size;
+};
+
+struct ps3_storage_device {
+ struct ps3_system_bus_device sbd;
+
+ struct ps3_dma_region dma_region;
+ unsigned int irq;
+ u64 blk_size;
+
+ u64 tag;
+ u64 lv1_status;
+ struct completion done;
+
+ unsigned long bounce_size;
+ void *bounce_buf;
+ u64 bounce_lpar;
+ dma_addr_t bounce_dma;
+
+ unsigned int num_regions;
+ unsigned long accessible_regions;
+ unsigned int region_idx; /* first accessible region */
+ struct ps3_storage_region regions[0]; /* Must be last */
+};
+
+static inline struct ps3_storage_device *to_ps3_storage_device(struct device *dev)
+{
+ return container_of(dev, struct ps3_storage_device, sbd.core);
+}
+
+extern int ps3stor_setup(struct ps3_storage_device *dev,
+ irq_handler_t handler);
+extern void ps3stor_teardown(struct ps3_storage_device *dev);
+extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+ u64 start_sector, u64 sectors,
+ int write);
+extern u64 ps3stor_send_command(struct ps3_storage_device *dev, u64 cmd,
+ u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+
+#endif /* _ASM_POWERPC_PS3STOR_H_ */
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 3/6] ps3: Storage device registration routines.
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-04 13:22 ` [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver Geert Uytterhoeven
2007-07-04 13:22 ` [patch 2/6] ps3: Storage Driver Core Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-04 13:22 ` [patch 4/6] ps3: Disk Storage Driver Geert Uytterhoeven
` (4 subsequent siblings)
7 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geert Uytterhoeven, Geoff Levand
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_probe.diff --]
[-- Type: text/plain, Size: 9357 bytes --]
Add support for storage devices to the device probe code.
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
v2:
- use msleep() instead of schedule_timeout()
arch/powerpc/platforms/ps3/device-init.c | 286 +++++++++++++++++++++++++++++++
1 files changed, 286 insertions(+)
--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -26,6 +26,7 @@
#include <asm/firmware.h>
#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
#include "platform.h"
@@ -237,6 +238,262 @@ static int __init ps3_setup_vuart_device
return result;
}
+static int ps3stor_wait_for_completion(u64 dev_id, u64 tag,
+ unsigned int timeout)
+{
+ int result = -1;
+ unsigned int retries = 0;
+ u64 status;
+
+ for (retries = 0; retries < timeout; retries++) {
+ result = lv1_storage_check_async_status(dev_id, tag, &status);
+ if (!result)
+ break;
+
+ msleep(1);
+ }
+
+ if (result)
+ pr_debug("%s:%u: check_async_status: %s, status %lx\n",
+ __func__, __LINE__, ps3_result(result), status);
+
+ return result;
+}
+
+/**
+ * ps3_storage_wait_for_device - Wait for a storage device to become ready.
+ * @repo: The repository device to wait for.
+ *
+ * Uses the hypervisor's storage device notification mechanism to wait until
+ * a storage device is ready. The device notification mechanism uses a
+ * psuedo device (id = -1) to asynchronously notify the guest when storage
+ * devices become ready. The notification device has a block size of 512
+ * bytes.
+ */
+
+static int ps3_storage_wait_for_device(const struct ps3_repository_device *repo)
+{
+ int result;
+ const u64 notification_dev_id = (u64)-1LL;
+ const unsigned int timeout = HZ;
+ u64 lpar;
+ u64 tag;
+ struct {
+ u64 operation_code; /* must be zero */
+ u64 event_mask; /* 1 = device ready */
+ } *notify_cmd;
+ struct {
+ u64 event_type; /* notify_device_ready */
+ u64 bus_id;
+ u64 dev_id;
+ u64 dev_type;
+ u64 dev_port;
+ } *notify_event;
+ enum {
+ notify_device_ready = 1
+ };
+
+ pr_debug(" -> %s:%u: bus_id %u, dev_id %u, dev_type %u\n", __func__,
+ __LINE__, repo->bus_id, repo->dev_id, repo->dev_type);
+
+ notify_cmd = kzalloc(512, GFP_KERNEL);
+ notify_event = (void *)notify_cmd;
+ if (!notify_cmd)
+ return -ENOMEM;
+
+ lpar = ps3_mm_phys_to_lpar(__pa(notify_cmd));
+
+ result = lv1_open_device(repo->bus_id, notification_dev_id, 0);
+ if (result) {
+ printk(KERN_ERR "%s:%u: lv1_open_device %s\n", __func__,
+ __LINE__, ps3_result(result));
+ result = -ENODEV;
+ goto fail_free;
+ }
+
+ /* Setup and write the request for device notification. */
+
+ notify_cmd->operation_code = 0; /* must be zero */
+ notify_cmd->event_mask = 0x01; /* device ready */
+
+ result = lv1_storage_write(notification_dev_id, 0, 0, 1, 0, lpar,
+ &tag);
+ if (result) {
+ printk(KERN_ERR "%s:%u: write failed %s\n", __func__, __LINE__,
+ ps3_result(result));
+ result = -ENODEV;
+ goto fail_close;
+ }
+
+ /* Wait for the write completion */
+
+ result = ps3stor_wait_for_completion(notification_dev_id, tag,
+ timeout);
+ if (result) {
+ printk(KERN_ERR "%s:%u: write not completed %s\n", __func__,
+ __LINE__, ps3_result(result));
+ result = -ENODEV;
+ goto fail_close;
+ }
+
+ /* Loop here processing the requested notification events. */
+
+ result = -ENODEV;
+ while (1) {
+ memset(notify_event, 0, sizeof(*notify_event));
+
+ result = lv1_storage_read(notification_dev_id, 0, 0, 1, 0,
+ lpar, &tag);
+ if (result) {
+ printk(KERN_ERR "%s:%u: write failed %s\n", __func__,
+ __LINE__, ps3_result(result));
+ break;
+ }
+
+ result = ps3stor_wait_for_completion(notification_dev_id, tag,
+ timeout);
+ if (result) {
+ printk(KERN_ERR "%s:%u: read not completed %s\n",
+ __func__, __LINE__, ps3_result(result));
+ break;
+ }
+
+ if (notify_event->event_type != notify_device_ready ||
+ notify_event->bus_id != repo->bus_id) {
+ pr_debug("%s:%u: bad notify_event: event %lu, "
+ "dev_id %lu, dev_type %lu\n",
+ __func__, __LINE__, notify_event->event_type,
+ notify_event->dev_id, notify_event->dev_type);
+ break;
+ }
+
+ if (notify_event->dev_id == repo->dev_id &&
+ notify_event->dev_type == repo->dev_type) {
+ pr_debug("%s:%u: device ready: dev_id %u\n", __func__,
+ __LINE__, repo->dev_id);
+ result = 0;
+ break;
+ }
+
+ if (notify_event->dev_id == repo->dev_id &&
+ notify_event->dev_type == PS3_DEV_TYPE_NOACCESS) {
+ pr_debug("%s:%u: no access: dev_id %u\n", __func__,
+ __LINE__, repo->dev_id);
+ break;
+ }
+ }
+
+fail_close:
+ lv1_close_device(repo->bus_id, notification_dev_id);
+fail_free:
+ kfree(notify_cmd);
+ pr_debug(" <- %s:%u\n", __func__, __LINE__);
+ return result;
+}
+
+static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
+ enum ps3_match_id match_id)
+{
+ int result;
+ struct ps3_storage_device *p;
+ u64 port, blk_size, num_blocks;
+ unsigned int num_regions, i;
+
+ pr_debug(" -> %s:%u: match_id %u\n", __func__, __LINE__, match_id);
+
+ result = ps3_repository_read_stor_dev_info(repo->bus_index,
+ repo->dev_index, &port,
+ &blk_size, &num_blocks,
+ &num_regions);
+ if (result) {
+ printk(KERN_ERR "%s:%u: _read_stor_dev_info failed %d\n",
+ __func__, __LINE__, result);
+ return -ENODEV;
+ }
+
+ pr_debug("%s:%u: index %u:%u: port %lu blk_size %lu num_blocks %lu "
+ "num_regions %u\n", __func__, __LINE__, repo->bus_index,
+ repo->dev_index, port, blk_size, num_blocks, num_regions);
+
+ p = kzalloc(sizeof(struct ps3_storage_device) +
+ num_regions * sizeof(struct ps3_storage_region),
+ GFP_KERNEL);
+ if (!p) {
+ result = -ENOMEM;
+ goto fail_malloc;
+ }
+
+ p->sbd.match_id = match_id;
+ p->sbd.dev_type = PS3_DEVICE_TYPE_SB;
+ p->sbd.bus_id = repo->bus_id;
+ p->sbd.dev_id = repo->dev_id;
+ p->sbd.d_region = &p->dma_region;
+ p->blk_size = blk_size;
+ p->num_regions = num_regions;
+
+ result = ps3_repository_find_interrupt(repo,
+ PS3_INTERRUPT_TYPE_EVENT_PORT,
+ &p->sbd.interrupt_id);
+ if (result) {
+ printk(KERN_ERR "%s:%u: find_interrupt failed %d\n", __func__,
+ __LINE__, result);
+ result = -ENODEV;
+ goto fail_find_interrupt;
+ }
+
+ /* FIXME: Arrange to only do this on a 'cold' boot */
+
+ result = ps3_storage_wait_for_device(repo);
+ if (result) {
+ printk(KERN_ERR "%s:%u: storage_notification failed %d\n",
+ __func__, __LINE__, result);
+ result = -ENODEV;
+ goto fail_probe_notification;
+ }
+
+ for (i = 0; i < num_regions; i++) {
+ unsigned int id;
+ u64 start, size;
+
+ result = ps3_repository_read_stor_dev_region(repo->bus_index,
+ repo->dev_index,
+ i, &id, &start,
+ &size);
+ if (result) {
+ printk(KERN_ERR
+ "%s:%u: read_stor_dev_region failed %d\n",
+ __func__, __LINE__, result);
+ result = -ENODEV;
+ goto fail_read_region;
+ }
+ pr_debug("%s:%u: region %u: id %u start %lu size %lu\n",
+ __func__, __LINE__, i, id, start, size);
+
+ p->regions[i].id = id;
+ p->regions[i].start = start;
+ p->regions[i].size = size;
+ }
+
+ result = ps3_system_bus_device_register(&p->sbd);
+ if (result) {
+ pr_debug("%s:%u ps3_system_bus_device_register failed\n",
+ __func__, __LINE__);
+ goto fail_device_register;
+ }
+
+ pr_debug(" <- %s:%u\n", __func__, __LINE__);
+ return 0;
+
+fail_device_register:
+fail_read_region:
+fail_probe_notification:
+fail_find_interrupt:
+ kfree(p);
+fail_malloc:
+ pr_debug(" <- %s:%u: fail.\n", __func__, __LINE__);
+ return result;
+}
+
static int __init ps3_register_vuart_devices(void)
{
int result;
@@ -356,6 +613,35 @@ static int ps3_register_repository_devic
__func__, __LINE__);
}
break;
+ case PS3_DEV_TYPE_STOR_DISK:
+ result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_DISK);
+
+ /* Some devices are not accessable from the Other OS lpar. */
+ if (result == -ENODEV) {
+ result = 0;
+ pr_debug("%s:%u: not accessable\n", __func__,
+ __LINE__);
+ }
+
+ if (result)
+ pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+ __func__, __LINE__);
+ break;
+
+ case PS3_DEV_TYPE_STOR_ROM:
+ result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_ROM);
+ if (result)
+ pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+ __func__, __LINE__);
+ break;
+
+ case PS3_DEV_TYPE_STOR_FLASH:
+ result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_FLASH);
+ if (result)
+ pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+ __func__, __LINE__);
+ break;
+
default:
result = 0;
pr_debug("%s:%u: unsupported dev_type %u\n", __func__, __LINE__,
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 4/6] ps3: Disk Storage Driver
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
` (2 preceding siblings ...)
2007-07-04 13:22 ` [patch 3/6] ps3: Storage device registration routines Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-04 13:22 ` [patch 5/6] ps3: BD/DVD/CD-ROM " Geert Uytterhoeven
` (3 subsequent siblings)
7 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geoff Levand, Geert Uytterhoeven
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_disk.diff --]
[-- Type: text/plain, Size: 18633 bytes --]
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Add a Disk Storage Driver for the PS3:
- Implemented as a block device driver with a dynamic major
- Disk names (and partitions) are of the format ps3d%c(%u)
- Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
doesn't support scatter-gather
CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
v3:
- Cleanup #includes
- Kill PS3DISK_MAJOR, always use zero to obtain a dynamic major
- Group LV1 API related stuff
- Kill empty ps3disk_open
- Reset ps3disk_priv(dev) to NULL during cleanup
- Move call to put_disk() down
- Pass the interrupt handler to ps3stor_setup(), so it doesn't have to be
overridden later
- Fold ps3disk_read_write_sectors() into ps3disk_submit_request_sg()
- Identify the hard drive capacity and model name during probing
v2:
- Don't use `default y' in Kconfig
- Remove the thread for handling requests
- Set up sysfs links between block device and PS3 system device:
o /sys/block/ps3da/device -> ../../devices/ps3_system/sb_02
o /sys/devices/ps3_system/sb_02/block:ps3da -> ../../../block/ps3da
- Fix all FIXMEs
- Implement proper barrier support
- Call add_disk_randomness()
- Add a test to verify we are running on a PS3
- Fix error path in ps3disk_init()
- Fix cleanup order in ps3disk_exit()
arch/powerpc/platforms/ps3/Kconfig | 10
drivers/block/Makefile | 1
drivers/block/ps3disk.c | 623 +++++++++++++++++++++++++++++++++++++
3 files changed, 634 insertions(+)
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -102,4 +102,14 @@ config PS3_STORAGE
depends on PPC_PS3
tristate
+config PS3_DISK
+ tristate "PS3 Disk Storage Driver"
+ depends on PPC_PS3 && BLOCK
+ select PS3_STORAGE
+ help
+ Include support for the PS3 Disk Storage.
+
+ This support is required to access the PS3 hard disk.
+ In general, all users will say Y or M.
+
endmenu
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_VIODASD) += viodasd.o
obj-$(CONFIG_BLK_DEV_SX8) += sx8.o
obj-$(CONFIG_BLK_DEV_UB) += ub.o
+obj-$(CONFIG_PS3_DISK) += ps3disk.o
--- /dev/null
+++ b/drivers/block/ps3disk.c
@@ -0,0 +1,623 @@
+/*
+ * PS3 Disk Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/ata.h>
+#include <linux/blkdev.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+#include <asm/firmware.h>
+
+
+#define DEVICE_NAME "ps3disk"
+
+#define BOUNCE_SIZE (64*1024)
+
+#define PS3DISK_MAX_DISKS 16
+#define PS3DISK_MINORS 16
+
+#define KERNEL_SECTOR_SIZE 512
+
+
+#define PS3DISK_NAME "ps3d%c"
+
+
+struct ps3disk_private {
+ spinlock_t lock; /* Request queue spinlock */
+ struct request_queue *queue;
+ struct gendisk *gendisk;
+ unsigned int blocking_factor;
+ struct request *req;
+ u64 raw_capacity;
+ unsigned char model[ATA_ID_PROD_LEN+1];
+};
+#define ps3disk_priv(dev) ((dev)->sbd.core.driver_data)
+
+
+#define LV1_STORAGE_SEND_ATA_COMMAND (2)
+#define LV1_STORAGE_ATA_HDDOUT (0x23)
+
+struct lv1_ata_cmnd_block {
+ u16 features;
+ u16 sector_count;
+ u16 LBA_low;
+ u16 LBA_mid;
+ u16 LBA_high;
+ u8 device;
+ u8 command;
+ u32 is_ext;
+ u32 proto;
+ u32 in_out;
+ u32 size;
+ u64 buffer;
+ u32 arglen;
+};
+
+enum lv1_ata_proto {
+ NON_DATA_PROTO = 0,
+ PIO_DATA_IN_PROTO = 1,
+ PIO_DATA_OUT_PROTO = 2,
+ DMA_PROTO = 3
+};
+
+enum lv1_ata_in_out {
+ DIR_WRITE = 0, /* memory -> device */
+ DIR_READ = 1 /* device -> memory */
+};
+
+static int ps3disk_major;
+
+
+static struct block_device_operations ps3disk_fops = {
+ .owner = THIS_MODULE,
+};
+
+
+static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
+ struct request *req, int gather)
+{
+ unsigned int sectors = 0, offset = 0;
+ struct bio *bio;
+ sector_t sector;
+ struct bio_vec *bvec;
+ unsigned int i = 0, j;
+ size_t size;
+ void *buf;
+
+ rq_for_each_bio(bio, req) {
+ sector = bio->bi_sector;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: bio %u: %u segs %u sectors from %lu\n",
+ __func__, __LINE__, i, bio_segments(bio),
+ bio_sectors(bio), sector);
+ bio_for_each_segment(bvec, bio, j) {
+ size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
+ buf = __bio_kmap_atomic(bio, j, KM_USER0);
+ if (gather)
+ memcpy(dev->bounce_buf+offset, buf, size);
+ else
+ memcpy(buf, dev->bounce_buf+offset, size);
+ offset += size;
+ __bio_kunmap_atomic(bio, KM_USER0);
+ }
+ sectors += bio_sectors(bio);
+ i++;
+ }
+}
+
+static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
+ struct request *req)
+{
+ struct ps3disk_private *priv = ps3disk_priv(dev);
+ int write = rq_data_dir(req), res;
+ const char *op = write ? "write" : "read";
+ u64 start_sector, sectors;
+ unsigned int region_id = dev->regions[dev->region_idx].id;
+
+#ifdef DEBUG
+ unsigned int n = 0;
+ struct bio *bio;
+ rq_for_each_bio(bio, req)
+ n++;
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: %s req has %u bios for %lu sectors %lu hard sectors\n",
+ __func__, __LINE__, op, n, req->nr_sectors,
+ req->hard_nr_sectors);
+#endif
+
+ start_sector = req->sector*priv->blocking_factor;
+ sectors = req->nr_sectors*priv->blocking_factor;
+ dev_dbg(&dev->sbd.core, "%s:%u: %s %lu sectors starting at %lu\n",
+ __func__, __LINE__, op, sectors, start_sector);
+
+ if (write) {
+ ps3disk_scatter_gather(dev, req, 1);
+
+ res = lv1_storage_write(dev->sbd.dev_id, region_id,
+ start_sector, sectors, 0,
+ dev->bounce_lpar, &dev->tag);
+ } else {
+ res = lv1_storage_read(dev->sbd.dev_id, region_id,
+ start_sector, sectors, 0,
+ dev->bounce_lpar, &dev->tag);
+ }
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+ __LINE__, op, res);
+ end_request(req, 0);
+ return 0;
+ }
+
+ priv->req = req;
+ return 1;
+}
+
+static int ps3disk_submit_flush_request(struct ps3_storage_device *dev,
+ struct request *req)
+{
+ struct ps3disk_private *priv = ps3disk_priv(dev);
+ u64 res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: flush request\n", __func__, __LINE__);
+
+ res = lv1_storage_send_device_command(dev->sbd.dev_id,
+ LV1_STORAGE_ATA_HDDOUT, 0, 0, 0,
+ 0, &dev->tag);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%lx\n",
+ __func__, __LINE__, res);
+ end_request(req, 0);
+ return 0;
+ }
+
+ priv->req = req;
+ return 1;
+}
+
+static void ps3disk_do_request(struct ps3_storage_device *dev,
+ request_queue_t *q)
+{
+ struct request *req;
+
+ dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+ while ((req = elv_next_request(q))) {
+ if (blk_fs_request(req)) {
+ if (ps3disk_submit_request_sg(dev, req))
+ break;
+ } else if (req->cmd_type == REQ_TYPE_FLUSH) {
+ if (ps3disk_submit_flush_request(dev, req))
+ break;
+ } else {
+ blk_dump_rq_flags(req, DEVICE_NAME " bad request");
+ end_request(req, 0);
+ continue;
+ }
+ }
+}
+
+static void ps3disk_request(request_queue_t *q)
+{
+ struct ps3_storage_device *dev = q->queuedata;
+ struct ps3disk_private *priv = ps3disk_priv(dev);
+
+ if (priv->req) {
+ dev_dbg(&dev->sbd.core, "%s:%u busy\n", __func__, __LINE__);
+ return;
+ }
+
+ ps3disk_do_request(dev, q);
+}
+
+static irqreturn_t ps3disk_interrupt(int irq, void *data)
+{
+ struct ps3_storage_device *dev = data;
+ struct ps3disk_private *priv;
+ struct request *req;
+ int res, read, uptodate;
+ u64 tag, status;
+ unsigned long num_sectors;
+ const char *op;
+
+ res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+ if (tag != dev->tag)
+ dev_err(&dev->sbd.core,
+ "%s:%u: tag mismatch, got %lx, expected %lx\n",
+ __func__, __LINE__, tag, dev->tag);
+
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+ __func__, __LINE__, res, status);
+ return IRQ_HANDLED;
+ }
+
+ priv = ps3disk_priv(dev);
+ req = priv->req;
+ if (!req) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u non-block layer request completed\n", __func__,
+ __LINE__);
+ dev->lv1_status = status;
+ complete(&dev->done);
+ return IRQ_HANDLED;
+ }
+
+ if (req->cmd_type == REQ_TYPE_FLUSH) {
+ read = 0;
+ num_sectors = req->hard_cur_sectors;
+ op = "flush";
+ } else {
+ read = !rq_data_dir(req);
+ num_sectors = req->nr_sectors;
+ op = read ? "read" : "write";
+ }
+ if (status) {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+ __LINE__, op, status);
+ uptodate = 0;
+ } else {
+ dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__,
+ __LINE__, op);
+ uptodate = 1;
+ if (read)
+ ps3disk_scatter_gather(dev, req, 0);
+ }
+
+ spin_lock(&priv->lock);
+ if (!end_that_request_first(req, uptodate, num_sectors)) {
+ add_disk_randomness(req->rq_disk);
+ blkdev_dequeue_request(req);
+ end_that_request_last(req, uptodate);
+ }
+ priv->req = NULL;
+ ps3disk_do_request(dev, priv->queue);
+ spin_unlock(&priv->lock);
+
+ return IRQ_HANDLED;
+}
+
+static int ps3disk_sync_cache(struct ps3_storage_device *dev)
+{
+ u64 res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: sync cache\n", __func__, __LINE__);
+
+ res = ps3stor_send_command(dev, LV1_STORAGE_ATA_HDDOUT, 0, 0, 0, 0);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%lx\n",
+ __func__, __LINE__, res);
+ return -EIO;
+ }
+ return 0;
+}
+
+
+/* ATA helpers copied from drivers/ata/libata-core.c */
+
+static void swap_buf_le16(u16 *buf, unsigned int buf_words)
+{
+#ifdef __BIG_ENDIAN
+ unsigned int i;
+
+ for (i = 0; i < buf_words; i++)
+ buf[i] = le16_to_cpu(buf[i]);
+#endif /* __BIG_ENDIAN */
+}
+
+static u64 ata_id_n_sectors(const u16 *id)
+{
+ if (ata_id_has_lba(id)) {
+ if (ata_id_has_lba48(id))
+ return ata_id_u64(id, 100);
+ else
+ return ata_id_u32(id, 60);
+ } else {
+ if (ata_id_current_chs_valid(id))
+ return ata_id_u32(id, 57);
+ else
+ return id[1] * id[3] * id[6];
+ }
+}
+
+static void ata_id_string(const u16 *id, unsigned char *s, unsigned int ofs,
+ unsigned int len)
+{
+ unsigned int c;
+
+ while (len > 0) {
+ c = id[ofs] >> 8;
+ *s = c;
+ s++;
+
+ c = id[ofs] & 0xff;
+ *s = c;
+ s++;
+
+ ofs++;
+ len -= 2;
+ }
+}
+
+static void ata_id_c_string(const u16 *id, unsigned char *s, unsigned int ofs,
+ unsigned int len)
+{
+ unsigned char *p;
+
+ WARN_ON(!(len & 1));
+
+ ata_id_string(id, s, ofs, len - 1);
+
+ p = s + strnlen(s, len - 1);
+ while (p > s && p[-1] == ' ')
+ p--;
+ *p = '\0';
+}
+
+static int ps3disk_identify(struct ps3_storage_device *dev)
+{
+ struct ps3disk_private *priv = ps3disk_priv(dev);
+ struct lv1_ata_cmnd_block ata_cmnd;
+ u16 *id = dev->bounce_buf;
+ u64 res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: identify disk\n", __func__, __LINE__);
+
+ memset(&ata_cmnd, 0, sizeof(struct lv1_ata_cmnd_block));
+ ata_cmnd.command = ATA_CMD_ID_ATA;
+ ata_cmnd.sector_count = 1;
+ ata_cmnd.size = ata_cmnd.arglen = ATA_ID_WORDS * 2;
+ ata_cmnd.buffer = dev->bounce_lpar;
+ ata_cmnd.proto = PIO_DATA_IN_PROTO;
+ ata_cmnd.in_out = DIR_READ;
+
+ res = ps3stor_send_command(dev, LV1_STORAGE_SEND_ATA_COMMAND,
+ ps3_mm_phys_to_lpar(__pa(&ata_cmnd)),
+ sizeof(ata_cmnd), ata_cmnd.buffer,
+ ata_cmnd.arglen);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: identify disk failed 0x%lx\n",
+ __func__, __LINE__, res);
+ return -EIO;
+ }
+
+ swap_buf_le16(id, ATA_ID_WORDS);
+
+ /* All we're interested in are raw capacity and model name */
+ priv->raw_capacity = ata_id_n_sectors(id);
+ ata_id_c_string(id, priv->model, ATA_ID_PROD, sizeof(priv->model));
+ return 0;
+}
+
+static void ps3disk_prepare_flush(request_queue_t *q, struct request *req)
+{
+ struct ps3_storage_device *dev = q->queuedata;
+
+ dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+ memset(req->cmd, 0, sizeof(req->cmd));
+ req->cmd_type = REQ_TYPE_FLUSH;
+}
+
+static int ps3disk_issue_flush(request_queue_t *q, struct gendisk *gendisk,
+ sector_t *sector)
+{
+ struct ps3_storage_device *dev = q->queuedata;
+ struct request *req;
+ int res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+ req = blk_get_request(q, WRITE, __GFP_WAIT);
+ ps3disk_prepare_flush(q, req);
+ res = blk_execute_rq(q, gendisk, req, 0);
+ if (res)
+ dev_err(&dev->sbd.core, "%s:%u: flush request failed %d\n",
+ __func__, __LINE__, res);
+ blk_put_request(req);
+ return res;
+}
+
+
+static unsigned long ps3disk_mask;
+
+static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3disk_private *priv;
+ int error;
+ unsigned int devidx;
+ struct request_queue *queue;
+ struct gendisk *gendisk;
+
+ if (dev->blk_size < KERNEL_SECTOR_SIZE) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: cannot handle block size %lu\n", __func__,
+ __LINE__, dev->blk_size);
+ return -EINVAL;
+ }
+
+ BUILD_BUG_ON(PS3DISK_MAX_DISKS > BITS_PER_LONG);
+ devidx = find_first_zero_bit(&ps3disk_mask, PS3DISK_MAX_DISKS);
+ if (devidx >= PS3DISK_MAX_DISKS) {
+ dev_err(&dev->sbd.core, "%s:%u: Too many disks\n", __func__,
+ __LINE__);
+ return -ENOSPC;
+ }
+ __set_bit(devidx, &ps3disk_mask);
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ ps3disk_priv(dev) = priv;
+ spin_lock_init(&priv->lock);
+
+ dev->bounce_size = BOUNCE_SIZE;
+ dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+ if (!dev->bounce_buf) {
+ error = -ENOMEM;
+ goto fail_free_priv;
+ }
+
+ error = ps3stor_setup(dev, ps3disk_interrupt);
+ if (error)
+ goto fail_free_bounce;
+
+ ps3disk_identify(dev);
+
+ queue = blk_init_queue(ps3disk_request, &priv->lock);
+ if (!queue) {
+ dev_err(&dev->sbd.core, "%s:%u: blk_init_queue failed\n",
+ __func__, __LINE__);
+ error = -ENOMEM;
+ goto fail_teardown;
+ }
+
+ priv->queue = queue;
+ queue->queuedata = dev;
+
+ blk_queue_bounce_limit(queue, BLK_BOUNCE_HIGH);
+
+ blk_queue_max_sectors(queue, dev->bounce_size/KERNEL_SECTOR_SIZE);
+ blk_queue_segment_boundary(queue, -1UL);
+ blk_queue_dma_alignment(queue, dev->blk_size-1);
+ blk_queue_hardsect_size(queue, dev->blk_size);
+
+ blk_queue_issue_flush_fn(queue, ps3disk_issue_flush);
+ blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH,
+ ps3disk_prepare_flush);
+
+ blk_queue_max_phys_segments(queue, -1);
+ blk_queue_max_hw_segments(queue, -1);
+ blk_queue_max_segment_size(queue, dev->bounce_size);
+
+ gendisk = alloc_disk(PS3DISK_MINORS);
+ if (!gendisk) {
+ dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n", __func__,
+ __LINE__);
+ error = -ENOMEM;
+ goto fail_cleanup_queue;
+ }
+
+ priv->gendisk = gendisk;
+ gendisk->major = ps3disk_major;
+ gendisk->first_minor = devidx * PS3DISK_MINORS;
+ gendisk->fops = &ps3disk_fops;
+ gendisk->queue = queue;
+ gendisk->private_data = dev;
+ gendisk->driverfs_dev = &dev->sbd.core;
+ snprintf(gendisk->disk_name, sizeof(gendisk->disk_name), PS3DISK_NAME,
+ devidx+'a');
+ priv->blocking_factor = dev->blk_size/KERNEL_SECTOR_SIZE;
+ set_capacity(gendisk,
+ dev->regions[dev->region_idx].size*priv->blocking_factor);
+
+ dev_info(&dev->sbd.core,
+ "%s is a %s (%lu MiB total, %lu MiB for OtherOS)\n",
+ gendisk->disk_name, priv->model, priv->raw_capacity >> 11,
+ get_capacity(gendisk) >> 11);
+
+ add_disk(gendisk);
+ return 0;
+
+fail_cleanup_queue:
+ blk_cleanup_queue(queue);
+fail_teardown:
+ ps3stor_teardown(dev);
+fail_free_bounce:
+ kfree(dev->bounce_buf);
+fail_free_priv:
+ kfree(priv);
+ ps3disk_priv(dev) = NULL;
+fail:
+ __clear_bit(devidx, &ps3disk_mask);
+ return error;
+}
+
+static int ps3disk_remove(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3disk_private *priv = ps3disk_priv(dev);
+
+ __clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
+ &ps3disk_mask);
+ del_gendisk(priv->gendisk);
+ blk_cleanup_queue(priv->queue);
+ put_disk(priv->gendisk);
+ dev_notice(&dev->sbd.core, "Synchronizing disk cache\n");
+ ps3disk_sync_cache(dev);
+ ps3stor_teardown(dev);
+ kfree(dev->bounce_buf);
+ kfree(priv);
+ ps3disk_priv(dev) = NULL;
+ return 0;
+}
+
+static struct ps3_system_bus_driver ps3disk = {
+ .match_id = PS3_MATCH_ID_STOR_DISK,
+ .core.name = DEVICE_NAME,
+ .core.owner = THIS_MODULE,
+ .probe = ps3disk_probe,
+ .remove = ps3disk_remove,
+ .shutdown = ps3disk_remove,
+};
+
+
+static int __init ps3disk_init(void)
+{
+ int error;
+
+ if (!firmware_has_feature(FW_FEATURE_PS3_LV1))
+ return -ENODEV;
+
+ error = register_blkdev(0, DEVICE_NAME);
+ if (error <= 0) {
+ printk(KERN_ERR "%s:%u: register_blkdev failed %d\n", __func__,
+ __LINE__, error);
+ return error;
+ }
+ ps3disk_major = error;
+
+ pr_info("%s:%u: registered block device major %d\n", __func__,
+ __LINE__, ps3disk_major);
+
+ error = ps3_system_bus_driver_register(&ps3disk);
+ if (error)
+ unregister_blkdev(ps3disk_major, DEVICE_NAME);
+
+ return error;
+}
+
+static void __exit ps3disk_exit(void)
+{
+ ps3_system_bus_driver_unregister(&ps3disk);
+ unregister_blkdev(ps3disk_major, DEVICE_NAME);
+}
+
+module_init(ps3disk_init);
+module_exit(ps3disk_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 Disk Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_DISK);
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
` (3 preceding siblings ...)
2007-07-04 13:22 ` [patch 4/6] ps3: Disk Storage Driver Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-13 13:02 ` James Bottomley
2007-07-04 13:22 ` [patch 6/6] ps3: FLASH ROM " Geert Uytterhoeven
` (2 subsequent siblings)
7 siblings, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geoff Levand, Geert Uytterhoeven
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_rom.diff --]
[-- Type: text/plain, Size: 17659 bytes --]
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Add a BD/DVD/CD-ROM Storage Driver for the PS3:
- Implemented as a SCSI device driver
- Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
doesn't support scatter-gather
CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
v3:
- Replace dependency on BLK_DEV_SR by SCSI
- Use `BD/DVD/CD-ROM' instead of `ROM' in driver descriptions
- Inform the user to enable "SCSI CDROM support"
- Cleanup #includes
- Group LV1 API related stuff
- Rename ps3rom_private.cmd to ps3rom_private.curr_cmd
- Use shost_priv() (from scsi-misc)
- Set scsi_device.use_10_for_rw and remove support for {READ,WRITE}_6
- Explain why we prefer to use lv1_storage_{read,write}()
- Pass the interrupt handler to ps3stor_setup(), so it doesn't have to be
overridden later
- Reset ps3rom_priv(dev) to NULL during cleanup
v2:
- Don't use `default y' in Kconfig
- Add missing #include <linux/highmem.h>
- Remove ps3rom_private.scsi_done, use scsi_cmnd.scsi_done instead
- Use scsi_device.host.hostdata
- Remove empty ps3rom_slave_{alloc,destroy}()
- Kill superfluous test for command in progress
- Move scsi_host_put() last in cleanup sequence
- Remove scsi_command(), use scsi_print_command() for debugging
- scsi_cmnd.use_sg is always > 0 these days
- Allocate struct ps3rom_private using scsi_host_alloc()
- Remove the thread for handling requests
- Remove unused position parameter enum
- Remove unused NA_PROTO and DIR_NA
- Derive buffer length, data direction, and transfer protocol from the
struct scsi_command, instead of using a big switch() statement
- Kill superfluous spinlock
- Remove manual request sense, as modern hypervisors always do auto sense
- Pass all SCSI commands to the hypervisor as ATAPI commands, except for
READ_*/WRITE_*
- Don't print errors for SCSI commands that are not allowed for an Other OS
by the hypervisor
- Remove superfluous tests for data directions in
{fill_from,fetch_to}_dev_buffer()
- Handle errors in {fill_from,fetch_to}_dev_buffer()
- Reorder routines
- Manually inline ps3rom_send_atapi_command()
- Fix all FIXMEs
arch/powerpc/platforms/ps3/Kconfig | 11
drivers/scsi/Makefile | 1
drivers/scsi/ps3rom.c | 537 +++++++++++++++++++++++++++++++++++++
3 files changed, 549 insertions(+)
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -112,4 +112,15 @@ config PS3_DISK
This support is required to access the PS3 hard disk.
In general, all users will say Y or M.
+config PS3_ROM
+ tristate "PS3 BD/DVD/CD-ROM Storage Driver"
+ depends on PPC_PS3 && SCSI
+ select PS3_STORAGE
+ help
+ Include support for the PS3 ROM Storage.
+
+ This support is required to access the PS3 BD/DVD/CD-ROM drive.
+ In general, all users will say Y or M.
+ Also make sure to say Y or M to "SCSI CDROM support" later.
+
endmenu
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -132,6 +132,7 @@ obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi/
obj-$(CONFIG_SCSI_IBMVSCSIS) += ibmvscsi/
obj-$(CONFIG_SCSI_HPTIOP) += hptiop.o
obj-$(CONFIG_SCSI_STEX) += stex.o
+obj-$(CONFIG_PS3_ROM) += ps3rom.o
obj-$(CONFIG_ARM) += arm/
--- /dev/null
+++ b/drivers/scsi/ps3rom.c
@@ -0,0 +1,537 @@
+/*
+ * PS3 BD/DVD/CD-ROM Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/cdrom.h>
+#include <linux/highmem.h>
+
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+
+
+#define DEVICE_NAME "ps3rom"
+
+#define BOUNCE_SIZE (64*1024)
+
+#define PS3ROM_MAX_SECTORS (BOUNCE_SIZE / CD_FRAMESIZE)
+
+
+struct ps3rom_private {
+ struct ps3_storage_device *dev;
+ struct scsi_cmnd *curr_cmd;
+};
+#define ps3rom_priv(dev) ((dev)->sbd.core.driver_data)
+
+
+#define LV1_STORAGE_SEND_ATAPI_COMMAND (1)
+
+struct lv1_atapi_cmnd_block {
+ u8 pkt[32]; /* packet command block */
+ u32 pktlen; /* should be 12 for ATAPI 8020 */
+ u32 blocks;
+ u32 block_size;
+ u32 proto; /* transfer mode */
+ u32 in_out; /* transfer direction */
+ u64 buffer; /* parameter except command block */
+ u32 arglen; /* length above */
+};
+
+enum lv1_atapi_proto {
+ NON_DATA_PROTO = 0,
+ PIO_DATA_IN_PROTO = 1,
+ PIO_DATA_OUT_PROTO = 2,
+ DMA_PROTO = 3
+};
+
+enum lv1_atapi_in_out {
+ DIR_WRITE = 0, /* memory -> device */
+ DIR_READ = 1 /* device -> memory */
+};
+
+
+static int ps3rom_slave_configure(struct scsi_device *scsi_dev)
+{
+ struct ps3rom_private *priv = shost_priv(scsi_dev->host);
+ struct ps3_storage_device *dev = priv->dev;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: id %u, lun %u, channel %u\n", __func__,
+ __LINE__, scsi_dev->id, scsi_dev->lun, scsi_dev->channel);
+
+ /*
+ * ATAPI SFF8020 devices use MODE_SENSE_10,
+ * so we can prohibit MODE_SENSE_6
+ */
+ scsi_dev->use_10_for_ms = 1;
+
+ /* we don't support {READ,WRITE}_6 */
+ scsi_dev->use_10_for_rw = 1;
+
+ return 0;
+}
+
+/*
+ * copy data from device into scatter/gather buffer
+ */
+static int fill_from_dev_buffer(struct scsi_cmnd *cmd, const void *buf)
+{
+ int k, req_len, act_len, len, active;
+ void *kaddr;
+ struct scatterlist *sgpnt;
+ unsigned int buflen;
+
+ buflen = cmd->request_bufflen;
+ if (!buflen)
+ return 0;
+
+ if (!cmd->request_buffer)
+ return -1;
+
+ sgpnt = cmd->request_buffer;
+ active = 1;
+ for (k = 0, req_len = 0, act_len = 0; k < cmd->use_sg; ++k, ++sgpnt) {
+ if (active) {
+ kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+ if (!kaddr)
+ return -1;
+ len = sgpnt->length;
+ if ((req_len + len) > buflen) {
+ active = 0;
+ len = buflen - req_len;
+ }
+ memcpy(kaddr + sgpnt->offset, buf + req_len, len);
+ kunmap_atomic(kaddr, KM_USER0);
+ act_len += len;
+ }
+ req_len += sgpnt->length;
+ }
+ cmd->resid = req_len - act_len;
+ return 0;
+}
+
+/*
+ * copy data from scatter/gather into device's buffer
+ */
+static int fetch_to_dev_buffer(struct scsi_cmnd *cmd, void *buf)
+{
+ int k, req_len, len, fin;
+ void *kaddr;
+ struct scatterlist *sgpnt;
+ unsigned int buflen;
+
+ buflen = cmd->request_bufflen;
+ if (!buflen)
+ return 0;
+
+ if (!cmd->request_buffer)
+ return -1;
+
+ sgpnt = cmd->request_buffer;
+ for (k = 0, req_len = 0, fin = 0; k < cmd->use_sg; ++k, ++sgpnt) {
+ kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+ if (!kaddr)
+ return -1;
+ len = sgpnt->length;
+ if ((req_len + len) > buflen) {
+ len = buflen - req_len;
+ fin = 1;
+ }
+ memcpy(buf + req_len, kaddr + sgpnt->offset, len);
+ kunmap_atomic(kaddr, KM_USER0);
+ if (fin)
+ return req_len + len;
+ req_len += sgpnt->length;
+ }
+ return req_len;
+}
+
+static int ps3rom_atapi_request(struct ps3_storage_device *dev,
+ struct scsi_cmnd *cmd)
+{
+ struct lv1_atapi_cmnd_block atapi_cmnd;
+ unsigned char opcode = cmd->cmnd[0];
+ int res;
+ u64 lpar;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: send ATAPI command 0x%02x\n", __func__,
+ __LINE__, opcode);
+
+ memset(&atapi_cmnd, 0, sizeof(struct lv1_atapi_cmnd_block));
+ memcpy(&atapi_cmnd.pkt, cmd->cmnd, 12);
+ atapi_cmnd.pktlen = 12;
+ atapi_cmnd.block_size = 1; /* transfer size is block_size * blocks */
+ atapi_cmnd.blocks = atapi_cmnd.arglen = cmd->request_bufflen;
+ atapi_cmnd.buffer = dev->bounce_lpar;
+
+ switch (cmd->sc_data_direction) {
+ case DMA_FROM_DEVICE:
+ if (cmd->request_bufflen >= CD_FRAMESIZE)
+ atapi_cmnd.proto = DMA_PROTO;
+ else
+ atapi_cmnd.proto = PIO_DATA_IN_PROTO;
+ atapi_cmnd.in_out = DIR_READ;
+ break;
+
+ case DMA_TO_DEVICE:
+ if (cmd->request_bufflen >= CD_FRAMESIZE)
+ atapi_cmnd.proto = DMA_PROTO;
+ else
+ atapi_cmnd.proto = PIO_DATA_OUT_PROTO;
+ atapi_cmnd.in_out = DIR_WRITE;
+ res = fetch_to_dev_buffer(cmd, dev->bounce_buf);
+ if (res < 0)
+ return DID_ERROR << 16;
+ break;
+
+ default:
+ atapi_cmnd.proto = NON_DATA_PROTO;
+ break;
+ }
+
+ lpar = ps3_mm_phys_to_lpar(__pa(&atapi_cmnd));
+ res = lv1_storage_send_device_command(dev->sbd.dev_id,
+ LV1_STORAGE_SEND_ATAPI_COMMAND,
+ lpar, sizeof(atapi_cmnd),
+ atapi_cmnd.buffer,
+ atapi_cmnd.arglen, &dev->tag);
+ if (res == LV1_DENIED_BY_POLICY) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: ATAPI command 0x%02x denied by policy\n",
+ __func__, __LINE__, opcode);
+ return DID_ERROR << 16;
+ }
+
+ if (res) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: ATAPI command 0x%02x failed %d\n", __func__,
+ __LINE__, opcode, res);
+ return DID_ERROR << 16;
+ }
+
+ return 0;
+}
+
+static inline unsigned int srb10_lba(const struct scsi_cmnd *cmd)
+{
+ return cmd->cmnd[2] << 24 | cmd->cmnd[3] << 16 | cmd->cmnd[4] << 8 |
+ cmd->cmnd[5];
+}
+
+static inline unsigned int srb10_len(const struct scsi_cmnd *cmd)
+{
+ return cmd->cmnd[7] << 8 | cmd->cmnd[8];
+}
+
+static int ps3rom_read_request(struct ps3_storage_device *dev,
+ struct scsi_cmnd *cmd, u32 start_sector,
+ u32 sectors)
+{
+ int res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: read %u sectors starting at %u\n",
+ __func__, __LINE__, sectors, start_sector);
+
+ res = lv1_storage_read(dev->sbd.dev_id,
+ dev->regions[dev->region_idx].id, start_sector,
+ sectors, 0, dev->bounce_lpar, &dev->tag);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: read failed %d\n", __func__,
+ __LINE__, res);
+ return DID_ERROR << 16;
+ }
+
+ return 0;
+}
+
+static int ps3rom_write_request(struct ps3_storage_device *dev,
+ struct scsi_cmnd *cmd, u32 start_sector,
+ u32 sectors)
+{
+ int res;
+
+ dev_dbg(&dev->sbd.core, "%s:%u: write %u sectors starting at %u\n",
+ __func__, __LINE__, sectors, start_sector);
+
+ res = fetch_to_dev_buffer(cmd, dev->bounce_buf);
+ if (res < 0)
+ return DID_ERROR << 16;
+
+ res = lv1_storage_write(dev->sbd.dev_id,
+ dev->regions[dev->region_idx].id, start_sector,
+ sectors, 0, dev->bounce_lpar, &dev->tag);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: write failed %d\n", __func__,
+ __LINE__, res);
+ return DID_ERROR << 16;
+ }
+
+ return 0;
+}
+
+static int ps3rom_queuecommand(struct scsi_cmnd *cmd,
+ void (*done)(struct scsi_cmnd *))
+{
+ struct ps3rom_private *priv = shost_priv(cmd->device->host);
+ struct ps3_storage_device *dev = priv->dev;
+ unsigned char opcode;
+ int res;
+
+#ifdef DEBUG
+ scsi_print_command(cmd);
+#endif
+
+ priv->curr_cmd = cmd;
+ cmd->scsi_done = done;
+
+ opcode = cmd->cmnd[0];
+ /*
+ * While we can submit READ/WRITE SCSI commands as ATAPI commands,
+ * it's recommended for various reasons (performance, error handling,
+ * ...) to use lv1_storage_{read,write}() instead
+ */
+ switch (opcode) {
+ case READ_10:
+ res = ps3rom_read_request(dev, cmd, srb10_lba(cmd),
+ srb10_len(cmd));
+ break;
+
+ case WRITE_10:
+ res = ps3rom_write_request(dev, cmd, srb10_lba(cmd),
+ srb10_len(cmd));
+ break;
+
+ default:
+ res = ps3rom_atapi_request(dev, cmd);
+ break;
+ }
+
+ if (res) {
+ memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
+ cmd->result = res;
+ cmd->sense_buffer[0] = 0x70;
+ cmd->sense_buffer[2] = ILLEGAL_REQUEST;
+ priv->curr_cmd = NULL;
+ cmd->scsi_done(cmd);
+ }
+
+ return 0;
+}
+
+static int decode_lv1_status(u64 status, unsigned char *sense_key,
+ unsigned char *asc, unsigned char *ascq)
+{
+ if (((status >> 24) & 0xff) != SAM_STAT_CHECK_CONDITION)
+ return -1;
+
+ *sense_key = (status >> 16) & 0xff;
+ *asc = (status >> 8) & 0xff;
+ *ascq = status & 0xff;
+ return 0;
+}
+
+static irqreturn_t ps3rom_interrupt(int irq, void *data)
+{
+ struct ps3_storage_device *dev = data;
+ struct Scsi_Host *host;
+ struct ps3rom_private *priv;
+ struct scsi_cmnd *cmd;
+ int res;
+ u64 tag, status;
+ unsigned char sense_key, asc, ascq;
+
+ res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+ /*
+ * status = -1 may mean that ATAPI transport completed OK, but
+ * ATAPI command itself resulted CHECK CONDITION
+ * so, upper layer should issue REQUEST_SENSE to check the sense data
+ */
+
+ if (tag != dev->tag)
+ dev_err(&dev->sbd.core,
+ "%s:%u: tag mismatch, got %lx, expected %lx\n",
+ __func__, __LINE__, tag, dev->tag);
+
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+ __func__, __LINE__, res, status);
+ return IRQ_HANDLED;
+ }
+
+ host = ps3rom_priv(dev);
+ priv = shost_priv(host);
+ cmd = priv->curr_cmd;
+
+ if (!status) {
+ /* OK, completed */
+ if (cmd->sc_data_direction == DMA_FROM_DEVICE) {
+ res = fill_from_dev_buffer(cmd, dev->bounce_buf);
+ if (res) {
+ cmd->result = DID_ERROR << 16;
+ goto done;
+ }
+ }
+ cmd->result = DID_OK << 16;
+ goto done;
+ }
+
+ if (cmd->cmnd[0] == REQUEST_SENSE) {
+ /* SCSI spec says request sense should never get error */
+ dev_err(&dev->sbd.core, "%s:%u: end error without autosense\n",
+ __func__, __LINE__);
+ cmd->result = DID_ERROR << 16 | SAM_STAT_CHECK_CONDITION;
+ goto done;
+ }
+
+ if (decode_lv1_status(status, &sense_key, &asc, &ascq)) {
+ cmd->result = DID_ERROR << 16;
+ goto done;
+ }
+
+ cmd->sense_buffer[0] = 0x70;
+ cmd->sense_buffer[2] = sense_key;
+ cmd->sense_buffer[7] = 16 - 6;
+ cmd->sense_buffer[12] = asc;
+ cmd->sense_buffer[13] = ascq;
+ cmd->result = SAM_STAT_CHECK_CONDITION;
+
+done:
+ priv->curr_cmd = NULL;
+ cmd->scsi_done(cmd);
+ return IRQ_HANDLED;
+}
+
+static struct scsi_host_template ps3rom_host_template = {
+ .name = DEVICE_NAME,
+ .slave_configure = ps3rom_slave_configure,
+ .queuecommand = ps3rom_queuecommand,
+ .can_queue = 1,
+ .this_id = 7,
+ .sg_tablesize = SG_ALL,
+ .cmd_per_lun = 1,
+ .emulated = 1, /* only sg driver uses this */
+ .max_sectors = PS3ROM_MAX_SECTORS,
+ .use_clustering = ENABLE_CLUSTERING,
+ .module = THIS_MODULE,
+};
+
+
+static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ int error;
+ struct Scsi_Host *host;
+ struct ps3rom_private *priv;
+
+ if (dev->blk_size != CD_FRAMESIZE) {
+ dev_err(&dev->sbd.core,
+ "%s:%u: cannot handle block size %lu\n", __func__,
+ __LINE__, dev->blk_size);
+ return -EINVAL;
+ }
+
+ dev->bounce_size = BOUNCE_SIZE;
+ dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+ if (!dev->bounce_buf)
+ return -ENOMEM;
+
+ error = ps3stor_setup(dev, ps3rom_interrupt);
+ if (error)
+ goto fail_free_bounce;
+
+ host = scsi_host_alloc(&ps3rom_host_template,
+ sizeof(struct ps3rom_private));
+ if (!host) {
+ dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed\n",
+ __func__, __LINE__);
+ goto fail_teardown;
+ }
+
+ priv = shost_priv(host);
+ ps3rom_priv(dev) = host;
+ priv->dev = dev;
+
+ /* One device/LUN per SCSI bus */
+ host->max_id = 1;
+ host->max_lun = 1;
+
+ error = scsi_add_host(host, &dev->sbd.core);
+ if (error) {
+ dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed %d\n",
+ __func__, __LINE__, error);
+ error = -ENODEV;
+ goto fail_host_put;
+ }
+
+ scsi_scan_host(host);
+ return 0;
+
+fail_host_put:
+ scsi_host_put(host);
+ ps3rom_priv(dev) = NULL;
+fail_teardown:
+ ps3stor_teardown(dev);
+fail_free_bounce:
+ kfree(dev->bounce_buf);
+ return error;
+}
+
+static int ps3rom_remove(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct Scsi_Host *host = ps3rom_priv(dev);
+
+ scsi_remove_host(host);
+ ps3stor_teardown(dev);
+ scsi_host_put(host);
+ ps3rom_priv(dev) = NULL;
+ kfree(dev->bounce_buf);
+ return 0;
+}
+
+static struct ps3_system_bus_driver ps3rom = {
+ .match_id = PS3_MATCH_ID_STOR_ROM,
+ .core.name = DEVICE_NAME,
+ .core.owner = THIS_MODULE,
+ .probe = ps3rom_probe,
+ .remove = ps3rom_remove
+};
+
+
+static int __init ps3rom_init(void)
+{
+ return ps3_system_bus_driver_register(&ps3rom);
+}
+
+static void __exit ps3rom_exit(void)
+{
+ ps3_system_bus_driver_unregister(&ps3rom);
+}
+
+module_init(ps3rom_init);
+module_exit(ps3rom_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 BD/DVD/CD-ROM Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_ROM);
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* [patch 6/6] ps3: FLASH ROM Storage Driver
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
` (4 preceding siblings ...)
2007-07-04 13:22 ` [patch 5/6] ps3: BD/DVD/CD-ROM " Geert Uytterhoeven
@ 2007-07-04 13:22 ` Geert Uytterhoeven
2007-07-10 7:45 ` [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-13 12:27 ` PS3 Storage Driver O_DIRECT issue Olaf Hering
7 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-04 13:22 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi, Geoff Levand, Geert Uytterhoeven
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: ps3-stable/ps3stor_flash.diff --]
[-- Type: text/plain, Size: 14402 bytes --]
From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Add a FLASH ROM Storage Driver for the PS3:
- Implemented as a misc character device driver
- Uses a fixed 256 KiB buffer allocated from boot memory as the hypervisor
requires the writing of aligned 256 KiB blocks
CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
v3:
- Cleanup #includes
- Allow to disable ps3flash (and the preallocated 256 KiB buffer) using
`ps3flash=off'
- Move selection of write/read strings to error path
- Rename and move ps3stor_interrupt() to ps3flash_interrupt()
- Pass the interrupt handler to ps3stor_setup()
- Reset ps3flash_priv(dev) to NULL during cleanup
v2:
- Don't use `default y' in Kconfig
- #include <linux/uaccess.h> instead of <asm/uaccess.h>
- Set up sysfs links between misc character device and PS3 system device:
o /sys/class/misc/ps3flash/device -> ../../../devices/ps3_system/sb_01
o /sys/devices/ps3_system/sb_01/misc:ps3flash -> ../../../class/misc/ps3flash
arch/powerpc/platforms/ps3/Kconfig | 15 +
drivers/char/Makefile | 2
drivers/char/ps3flash.c | 429 +++++++++++++++++++++++++++++++++++++
3 files changed, 446 insertions(+)
--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -123,4 +123,19 @@ config PS3_ROM
In general, all users will say Y or M.
Also make sure to say Y or M to "SCSI CDROM support" later.
+config PS3_FLASH
+ tristate "PS3 FLASH ROM Storage Driver"
+ depends on PPC_PS3
+ select PS3_STORAGE
+ help
+ Include support for the PS3 FLASH ROM Storage.
+
+ This support is required to access the PS3 FLASH ROM, which
+ contains the boot loader and some boot options.
+ In general, all users will say Y or M.
+
+ As this driver needs a fixed buffer of 256 KiB of memory, it can
+ be disabled on the kernel command line using "ps3flash=off", to
+ not allocate this fixed buffer.
+
endmenu
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -104,6 +104,8 @@ obj-$(CONFIG_IPMI_HANDLER) += ipmi/
obj-$(CONFIG_HANGCHECK_TIMER) += hangcheck-timer.o
obj-$(CONFIG_TCG_TPM) += tpm/
+obj-$(CONFIG_PS3_FLASH) += ps3flash.o
+
# Files generated that shall be removed upon make clean
clean-files := consolemap_deftbl.c defkeymap.c
--- /dev/null
+++ b/drivers/char/ps3flash.c
@@ -0,0 +1,429 @@
+/*
+ * PS3 FLASH ROM Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+
+
+#define DEVICE_NAME "ps3flash"
+
+#define FLASH_BLOCK_SIZE (256*1024)
+
+
+struct ps3flash_private {
+ struct mutex mutex; /* Bounce buffer mutex */
+};
+#define ps3flash_priv(dev) ((dev)->sbd.core.driver_data)
+
+static struct ps3_storage_device *ps3flash_dev;
+
+static ssize_t ps3flash_read_write_sectors(struct ps3_storage_device *dev,
+ u64 lpar, u64 start_sector,
+ u64 sectors, int write)
+{
+ u64 res = ps3stor_read_write_sectors(dev, lpar, start_sector, sectors,
+ write);
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+ __LINE__, write ? "write" : "read", res);
+ return -EIO;
+ }
+ return sectors;
+}
+
+static ssize_t ps3flash_read_sectors(struct ps3_storage_device *dev,
+ u64 start_sector, u64 sectors,
+ unsigned int sector_offset)
+{
+ u64 max_sectors, lpar;
+
+ max_sectors = dev->bounce_size / dev->blk_size;
+ if (sectors > max_sectors) {
+ dev_dbg(&dev->sbd.core, "%s:%u Limiting sectors to %lu\n",
+ __func__, __LINE__, max_sectors);
+ sectors = max_sectors;
+ }
+
+ lpar = dev->bounce_lpar + sector_offset * dev->blk_size;
+ return ps3flash_read_write_sectors(dev, lpar, start_sector, sectors,
+ 0);
+}
+
+static ssize_t ps3flash_write_chunk(struct ps3_storage_device *dev,
+ u64 start_sector)
+{
+ u64 sectors = dev->bounce_size / dev->blk_size;
+ return ps3flash_read_write_sectors(dev, dev->bounce_lpar, start_sector,
+ sectors, 1);
+}
+
+static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
+{
+ struct ps3_storage_device *dev = ps3flash_dev;
+ u64 size = dev->regions[dev->region_idx].size*dev->blk_size;
+
+ switch (origin) {
+ case 1:
+ offset += file->f_pos;
+ break;
+ case 2:
+ offset += size;
+ break;
+ }
+ if (offset < 0)
+ return -EINVAL;
+
+ file->f_pos = offset;
+ return file->f_pos;
+}
+
+static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
+ loff_t *pos)
+{
+ struct ps3_storage_device *dev = ps3flash_dev;
+ struct ps3flash_private *priv = ps3flash_priv(dev);
+ u64 size, start_sector, end_sector, offset;
+ ssize_t sectors_read;
+ size_t remaining, n;
+
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: Reading %zu bytes at position %lld to user 0x%p\n",
+ __func__, __LINE__, count, *pos, buf);
+
+ size = dev->regions[dev->region_idx].size*dev->blk_size;
+ if (*pos >= size || !count)
+ return 0;
+
+ if (*pos+count > size) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u Truncating count from %zu to %llu\n", __func__,
+ __LINE__, count, size - *pos);
+ count = size - *pos;
+ }
+
+ start_sector = do_div_llr(*pos, dev->blk_size, &offset);
+ end_sector = DIV_ROUND_UP(*pos+count, dev->blk_size);
+
+ remaining = count;
+ do {
+ mutex_lock(&priv->mutex);
+
+ sectors_read = ps3flash_read_sectors(dev, start_sector,
+ end_sector-start_sector,
+ 0);
+ if (sectors_read < 0) {
+ mutex_unlock(&priv->mutex);
+ return sectors_read;
+ }
+
+ n = min(remaining, sectors_read*dev->blk_size-offset);
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: copy %lu bytes from 0x%p to user 0x%p\n",
+ __func__, __LINE__, n, dev->bounce_buf+offset, buf);
+ if (copy_to_user(buf, dev->bounce_buf+offset, n)) {
+ mutex_unlock(&priv->mutex);
+ return -EFAULT;
+ }
+
+ mutex_unlock(&priv->mutex);
+
+ *pos += n;
+ buf += n;
+ remaining -= n;
+ start_sector += sectors_read;
+ offset = 0;
+ } while (remaining > 0);
+
+ return count;
+}
+
+static ssize_t ps3flash_write(struct file *file, const char __user *buf,
+ size_t count, loff_t *pos)
+{
+ struct ps3_storage_device *dev = ps3flash_dev;
+ struct ps3flash_private *priv = ps3flash_priv(dev);
+ u64 size, chunk_sectors, start_write_sector, end_write_sector,
+ end_read_sector, start_read_sector, head, tail, offset;
+ ssize_t res;
+ size_t remaining, n;
+ unsigned int sec_off;
+
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: Writing %zu bytes at position %lld from user 0x%p\n",
+ __func__, __LINE__, count, *pos, buf);
+
+ size = dev->regions[dev->region_idx].size*dev->blk_size;
+ if (*pos >= size || !count)
+ return 0;
+
+ if (*pos+count > size) {
+ dev_dbg(&dev->sbd.core,
+ "%s:%u Truncating count from %zu to %llu\n", __func__,
+ __LINE__, count, size - *pos);
+ count = size - *pos;
+ }
+
+ chunk_sectors = dev->bounce_size / dev->blk_size;
+
+ start_write_sector = do_div_llr(*pos, dev->bounce_size, &offset) *
+ chunk_sectors;
+ end_write_sector = DIV_ROUND_UP(*pos+count, dev->bounce_size) *
+ chunk_sectors;
+
+ end_read_sector = DIV_ROUND_UP(*pos, dev->blk_size);
+ start_read_sector = (*pos+count) / dev->blk_size;
+
+ /*
+ * As we have to write in 256 KiB chunks, while we can read in blk_size
+ * (usually 512 bytes) chunks, we perform the following steps:
+ * 1. Read from start_write_sector to end_read_sector ("head")
+ * 2. Read from start_read_sector to end_write_sector ("tail")
+ * 3. Copy data to buffer
+ * 4. Write from start_write_sector to end_write_sector
+ * All of this is complicated by using only one 256 KiB bounce buffer.
+ */
+
+ head = end_read_sector-start_write_sector;
+ tail = end_write_sector-start_read_sector;
+
+ remaining = count;
+ do {
+ mutex_lock(&priv->mutex);
+
+ if (end_read_sector >= start_read_sector) {
+ /* Merge head and tail */
+ dev_dbg(&dev->sbd.core,
+ "Merged head and tail: %lu sectors at %lu\n",
+ chunk_sectors, start_write_sector);
+ res = ps3flash_read_sectors(dev, start_write_sector,
+ chunk_sectors, 0);
+ if (res < 0)
+ goto fail;
+ } else {
+ if (head) {
+ /* Read head */
+ dev_dbg(&dev->sbd.core,
+ "head: %lu sectors at %lu\n", head,
+ start_write_sector);
+ res = ps3flash_read_sectors(dev,
+ start_write_sector,
+ head, 0);
+ if (res < 0)
+ goto fail;
+ }
+ if (start_read_sector <
+ start_write_sector+chunk_sectors) {
+ /* Read tail */
+ dev_dbg(&dev->sbd.core,
+ "tail: %lu sectors at %lu\n", tail,
+ start_read_sector);
+ sec_off = start_read_sector-start_write_sector;
+ res = ps3flash_read_sectors(dev,
+ start_read_sector,
+ tail, sec_off);
+ if (res < 0)
+ goto fail;
+ }
+ }
+
+ n = min(remaining, dev->bounce_size-offset);
+ dev_dbg(&dev->sbd.core,
+ "%s:%u: copy %lu bytes from user 0x%p to 0x%p\n",
+ __func__, __LINE__, n, buf, dev->bounce_buf+offset);
+ if (copy_from_user(dev->bounce_buf+offset, buf, n)) {
+ res = -EFAULT;
+ goto fail;
+ }
+
+ res = ps3flash_write_chunk(dev, start_write_sector);
+ if (res < 0)
+ goto fail;
+
+ mutex_unlock(&priv->mutex);
+
+ *pos += n;
+ buf += n;
+ remaining -= n;
+ start_write_sector += chunk_sectors;
+ head = 0;
+ offset = 0;
+ } while (remaining > 0);
+
+ return count;
+
+fail:
+ mutex_unlock(&priv->mutex);
+ return res;
+}
+
+
+static irqreturn_t ps3flash_interrupt(int irq, void *data)
+{
+ struct ps3_storage_device *dev = data;
+ int res;
+ u64 tag, status;
+
+ res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+ if (tag != dev->tag)
+ dev_err(&dev->sbd.core,
+ "%s:%u: tag mismatch, got %lx, expected %lx\n",
+ __func__, __LINE__, tag, dev->tag);
+
+ if (res) {
+ dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+ __func__, __LINE__, res, status);
+ } else {
+ dev->lv1_status = status;
+ complete(&dev->done);
+ }
+ return IRQ_HANDLED;
+}
+
+
+static const struct file_operations ps3flash_fops = {
+ .owner = THIS_MODULE,
+ .llseek = ps3flash_llseek,
+ .read = ps3flash_read,
+ .write = ps3flash_write,
+};
+
+static struct miscdevice ps3flash_misc = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = DEVICE_NAME,
+ .fops = &ps3flash_fops,
+};
+
+static int __devinit ps3flash_probe(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+ struct ps3flash_private *priv;
+ int error;
+ unsigned long tmp;
+
+ tmp = dev->regions[dev->region_idx].start*dev->blk_size;
+ if (tmp % FLASH_BLOCK_SIZE) {
+ dev_err(&dev->sbd.core,
+ "%s:%u region start %lu is not aligned\n", __func__,
+ __LINE__, tmp);
+ return -EINVAL;
+ }
+ tmp = dev->regions[dev->region_idx].size*dev->blk_size;
+ if (tmp % FLASH_BLOCK_SIZE) {
+ dev_err(&dev->sbd.core,
+ "%s:%u region size %lu is not aligned\n", __func__,
+ __LINE__, tmp);
+ return -EINVAL;
+ }
+
+ /* use static buffer, kmalloc cannot allocate 256 KiB */
+ if (!ps3flash_bounce_buffer.address)
+ return -ENODEV;
+
+ if (ps3flash_dev) {
+ dev_err(&dev->sbd.core,
+ "Only one FLASH device is supported\n");
+ return -EBUSY;
+ }
+
+ ps3flash_dev = dev;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ error = -ENOMEM;
+ goto fail;
+ }
+
+ ps3flash_priv(dev) = priv;
+ mutex_init(&priv->mutex);
+
+ dev->bounce_size = ps3flash_bounce_buffer.size;
+ dev->bounce_buf = ps3flash_bounce_buffer.address;
+
+ error = ps3stor_setup(dev, ps3flash_interrupt);
+ if (error)
+ goto fail_free_priv;
+
+ ps3flash_misc.parent = &dev->sbd.core;
+ error = misc_register(&ps3flash_misc);
+ if (error) {
+ dev_err(&dev->sbd.core, "%s:%u: misc_register failed %d\n",
+ __func__, __LINE__, error);
+ goto fail_teardown;
+ }
+
+ dev_info(&dev->sbd.core, "%s:%u: registered misc device %d\n",
+ __func__, __LINE__, ps3flash_misc.minor);
+ return 0;
+
+fail_teardown:
+ ps3stor_teardown(dev);
+fail_free_priv:
+ kfree(priv);
+ ps3flash_priv(dev) = NULL;
+fail:
+ ps3flash_dev = NULL;
+ return error;
+}
+
+static int ps3flash_remove(struct ps3_system_bus_device *_dev)
+{
+ struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+
+ misc_deregister(&ps3flash_misc);
+ ps3stor_teardown(dev);
+ kfree(ps3flash_priv(dev));
+ ps3flash_priv(dev) = NULL;
+ ps3flash_dev = NULL;
+ return 0;
+}
+
+
+static struct ps3_system_bus_driver ps3flash = {
+ .match_id = PS3_MATCH_ID_STOR_FLASH,
+ .core.name = DEVICE_NAME,
+ .core.owner = THIS_MODULE,
+ .probe = ps3flash_probe,
+ .remove = ps3flash_remove,
+ .shutdown = ps3flash_remove,
+};
+
+
+static int __init ps3flash_init(void)
+{
+ return ps3_system_bus_driver_register(&ps3flash);
+}
+
+static void __exit ps3flash_exit(void)
+{
+ ps3_system_bus_driver_unregister(&ps3flash);
+}
+
+module_init(ps3flash_init);
+module_exit(ps3flash_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 FLASH ROM Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_FLASH);
--
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
` (5 preceding siblings ...)
2007-07-04 13:22 ` [patch 6/6] ps3: FLASH ROM " Geert Uytterhoeven
@ 2007-07-10 7:45 ` Geert Uytterhoeven
2007-07-10 7:55 ` Jens Axboe
2007-07-13 9:31 ` Geert Uytterhoeven
2007-07-13 12:27 ` PS3 Storage Driver O_DIRECT issue Olaf Hering
7 siblings, 2 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-10 7:45 UTC (permalink / raw)
To: Paul Mackerras, Jens Axboe, James E.J. Bottomley, Alessandro Rubini
Cc: linuxppc-dev, linux-kernel, linux-scsi
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1646 bytes --]
On Wed, 4 Jul 2007, Geert Uytterhoeven wrote:
> This is the fourth submission of the new PS3 storage drivers:
> [1] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
> [2] ps3: Storage Driver Core
> [3] ps3: Storage device registration routines.
> [4] ps3: Disk Storage Driver
> [5] ps3: BD/DVD/CD-ROM Storage Driver
> [6] ps3: FLASH ROM Storage Driver
>
> They are based on:
> - the current Linux kernel source tree,
> - plus the PS3 patches already submitted by Geoff Levand on linuxppc-dev,
> - plus the shost_priv() patch in scsi-misc (commit
> bcd92c9fbcc679ee95003083056f0441a1f474fa).
>
> All issues raised during the previous review rounds should be fixed.
> There were no more comments after the third submission.
>
> Paul already integrated Geoff Levand's PS3 patches and the first 3 patches
> in this series in the for-2.6.23 branch in powerpc.git.
>
> Maintainers of block/SCSI/misc, can you please ack the last 3 patches so Paul
> can take them, too?
Ping block/SCSI/misc maintainers?
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
2007-07-10 7:45 ` [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
@ 2007-07-10 7:55 ` Jens Axboe
2007-07-10 8:01 ` Paul Mackerras
2007-07-13 9:31 ` Geert Uytterhoeven
1 sibling, 1 reply; 46+ messages in thread
From: Jens Axboe @ 2007-07-10 7:55 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Paul Mackerras, James E.J. Bottomley, Alessandro Rubini,
linuxppc-dev, linux-kernel, linux-scsi
On Tue, Jul 10 2007, Geert Uytterhoeven wrote:
> On Wed, 4 Jul 2007, Geert Uytterhoeven wrote:
> > This is the fourth submission of the new PS3 storage drivers:
> > [1] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
> > [2] ps3: Storage Driver Core
> > [3] ps3: Storage device registration routines.
> > [4] ps3: Disk Storage Driver
> > [5] ps3: BD/DVD/CD-ROM Storage Driver
> > [6] ps3: FLASH ROM Storage Driver
> >
> > They are based on:
> > - the current Linux kernel source tree,
> > - plus the PS3 patches already submitted by Geoff Levand on linuxppc-dev,
> > - plus the shost_priv() patch in scsi-misc (commit
> > bcd92c9fbcc679ee95003083056f0441a1f474fa).
> >
> > All issues raised during the previous review rounds should be fixed.
> > There were no more comments after the third submission.
> >
> > Paul already integrated Geoff Levand's PS3 patches and the first 3 patches
> > in this series in the for-2.6.23 branch in powerpc.git.
> >
> > Maintainers of block/SCSI/misc, can you please ack the last 3 patches so Paul
> > can take them, too?
>
> Ping block/SCSI/misc maintainers?
I have no objections to the block bits, however I feel uneasy merging
the full patchset unless patch #1 has been acked by the platform person.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
2007-07-10 7:55 ` Jens Axboe
@ 2007-07-10 8:01 ` Paul Mackerras
2007-07-10 8:32 ` Jens Axboe
0 siblings, 1 reply; 46+ messages in thread
From: Paul Mackerras @ 2007-07-10 8:01 UTC (permalink / raw)
To: Jens Axboe
Cc: Geert Uytterhoeven, James E.J. Bottomley, Alessandro Rubini,
linuxppc-dev, linux-kernel, linux-scsi
Jens Axboe writes:
> I have no objections to the block bits, however I feel uneasy merging
> the full patchset unless patch #1 has been acked by the platform person.
I have the first 3 patches in my queue, so yes I've acked it. It's
probably easiest if the remaining 3 go through my queue once they've
been acked - or if you prefer, you can push #4 via your tree once I've
got Linus to pull my tree. Let me know either way.
Paul.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
2007-07-10 8:01 ` Paul Mackerras
@ 2007-07-10 8:32 ` Jens Axboe
0 siblings, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-10 8:32 UTC (permalink / raw)
To: Paul Mackerras
Cc: Geert Uytterhoeven, James E.J. Bottomley, Alessandro Rubini,
linuxppc-dev, linux-kernel, linux-scsi
On Tue, Jul 10 2007, Paul Mackerras wrote:
> Jens Axboe writes:
>
> > I have no objections to the block bits, however I feel uneasy merging
> > the full patchset unless patch #1 has been acked by the platform person.
>
> I have the first 3 patches in my queue, so yes I've acked it. It's
> probably easiest if the remaining 3 go through my queue once they've
> been acked - or if you prefer, you can push #4 via your tree once I've
> got Linus to pull my tree. Let me know either way.
If you already have the 3 first patches, just slurp up the rest as well.
No point in making it more complicated than we have to.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4
2007-07-10 7:45 ` [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-10 7:55 ` Jens Axboe
@ 2007-07-13 9:31 ` Geert Uytterhoeven
1 sibling, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 9:31 UTC (permalink / raw)
To: Paul Mackerras, James E.J. Bottomley, Alessandro Rubini
Cc: Linux/PPC Development, Linux Kernel Development, linux-scsi
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1901 bytes --]
On Tue, 10 Jul 2007, Geert Uytterhoeven wrote:
> On Wed, 4 Jul 2007, Geert Uytterhoeven wrote:
> > This is the fourth submission of the new PS3 storage drivers:
> > [1] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
> > [2] ps3: Storage Driver Core
> > [3] ps3: Storage device registration routines.
> > [4] ps3: Disk Storage Driver
> > [5] ps3: BD/DVD/CD-ROM Storage Driver
> > [6] ps3: FLASH ROM Storage Driver
> >
> > They are based on:
> > - the current Linux kernel source tree,
> > - plus the PS3 patches already submitted by Geoff Levand on linuxppc-dev,
> > - plus the shost_priv() patch in scsi-misc (commit
> > bcd92c9fbcc679ee95003083056f0441a1f474fa).
> >
> > All issues raised during the previous review rounds should be fixed.
> > There were no more comments after the third submission.
> >
> > Paul already integrated Geoff Levand's PS3 patches and the first 3 patches
> > in this series in the for-2.6.23 branch in powerpc.git.
> >
> > Maintainers of block/SCSI/misc, can you please ack the last 3 patches so Paul
> > can take them, too?
>
> Ping block/SCSI/misc maintainers?
Ping SCSI and misc maintainers?
Please either ack patches 5 and 6, so Paulus can integrate them for 2.6.23, or
tell me why they can't go in.
Thanks again.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* PS3 Storage Driver O_DIRECT issue
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
` (6 preceding siblings ...)
2007-07-10 7:45 ` [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
@ 2007-07-13 12:27 ` Olaf Hering
2007-07-13 13:34 ` Geert Uytterhoeven
2007-07-18 16:12 ` Geert Uytterhoeven
7 siblings, 2 replies; 46+ messages in thread
From: Olaf Hering @ 2007-07-13 12:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Paul Mackerras, Jens Axboe, James E.J. Bottomley,
Alessandro Rubini, linuxppc-dev, linux-kernel, linux-scsi
This driver (or the generic PS3 code) has appearently problems with
O_DIRECT.
glibc aborts parted because the malloc metadata get corrupted. While it
is reproducible, the place where it crashes changes with every version
of the debug attempt.
I dont have a handle right now, all I know is that the metadata after a
malloc area get overwritten with zeros.
Can you have a look at this?
parted /dev/ps3da
print (a few times)
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-04 13:22 ` [patch 5/6] ps3: BD/DVD/CD-ROM " Geert Uytterhoeven
@ 2007-07-13 13:02 ` James Bottomley
2007-07-13 13:05 ` Jens Axboe
2007-07-13 23:06 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 46+ messages in thread
From: James Bottomley @ 2007-07-13 13:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Paul Mackerras, Jens Axboe, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:
> + kaddr = kmap_atomic(sgpnt->page, KM_USER0);
> + if (!kaddr)
> + return -1;
> + len = sgpnt->length;
> + if ((req_len + len) > buflen) {
> + active = 0;
> + len = buflen - req_len;
> + }
> + memcpy(kaddr + sgpnt->offset, buf + req_len,
> len);
> + kunmap_atomic(kaddr, KM_USER0);
This isn't a SCSI objection, but this sequence appears several times in
this driver. It's wrong for a non-PIPT architecture (and I believe the
PS3 is VIPT) because you copy into the kernel alias for the page, which
dirties the line in the cache of that alias (the user alias cache line
was already invalidated). However, unless you flush the kernel alias to
main memory, the user could read stale data. The way this is supposed
to be done is to do a
flush_kernel_dcache_page(kaddr)
before doing the kunmap.
Otherwise it looks OK from the SCSI point of view.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:02 ` James Bottomley
@ 2007-07-13 13:05 ` Jens Axboe
2007-07-13 13:25 ` Geert Uytterhoeven
2007-07-13 23:06 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 46+ messages in thread
From: Jens Axboe @ 2007-07-13 13:05 UTC (permalink / raw)
To: James Bottomley
Cc: Geert Uytterhoeven, Paul Mackerras, Alessandro Rubini,
linuxppc-dev, linux-kernel, linux-scsi, Geoff Levand
On Fri, Jul 13 2007, James Bottomley wrote:
> On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:
> > + kaddr = kmap_atomic(sgpnt->page, KM_USER0);
> > + if (!kaddr)
> > + return -1;
> > + len = sgpnt->length;
> > + if ((req_len + len) > buflen) {
> > + active = 0;
> > + len = buflen - req_len;
> > + }
> > + memcpy(kaddr + sgpnt->offset, buf + req_len,
> > len);
> > + kunmap_atomic(kaddr, KM_USER0);
>
> This isn't a SCSI objection, but this sequence appears several times in
> this driver. It's wrong for a non-PIPT architecture (and I believe the
> PS3 is VIPT) because you copy into the kernel alias for the page, which
> dirties the line in the cache of that alias (the user alias cache line
> was already invalidated). However, unless you flush the kernel alias to
> main memory, the user could read stale data. The way this is supposed
> to be done is to do a
>
> flush_kernel_dcache_page(kaddr)
>
> before doing the kunmap.
>
> Otherwise it looks OK from the SCSI point of view.
Well, even worse is that fact that it's using KM_USER0 from interrupt
context.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:05 ` Jens Axboe
@ 2007-07-13 13:25 ` Geert Uytterhoeven
2007-07-13 13:34 ` James Bottomley
2007-07-13 14:51 ` Jens Axboe
0 siblings, 2 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 13:25 UTC (permalink / raw)
To: Jens Axboe
Cc: James Bottomley, Paul Mackerras, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2409 bytes --]
On Fri, 13 Jul 2007, Jens Axboe wrote:
> On Fri, Jul 13 2007, James Bottomley wrote:
> > On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:
> > > + kaddr = kmap_atomic(sgpnt->page, KM_USER0);
> > > + if (!kaddr)
> > > + return -1;
> > > + len = sgpnt->length;
> > > + if ((req_len + len) > buflen) {
> > > + active = 0;
> > > + len = buflen - req_len;
> > > + }
> > > + memcpy(kaddr + sgpnt->offset, buf + req_len,
> > > len);
> > > + kunmap_atomic(kaddr, KM_USER0);
> >
> > This isn't a SCSI objection, but this sequence appears several times in
> > this driver. It's wrong for a non-PIPT architecture (and I believe the
> > PS3 is VIPT) because you copy into the kernel alias for the page, which
> > dirties the line in the cache of that alias (the user alias cache line
> > was already invalidated). However, unless you flush the kernel alias to
> > main memory, the user could read stale data. The way this is supposed
> > to be done is to do a
> >
> > flush_kernel_dcache_page(kaddr)
> >
> > before doing the kunmap.
> >
> > Otherwise it looks OK from the SCSI point of view.
kmap() just returns page_address() on ppc64, as there's no highmem.
kunmap() is a no-op.
So technically I could just use page_address() directly, but Christoph wanted
me to keep the kmap()/kunmap() sequence because it's considered a good
practice.
> Well, even worse is that fact that it's using KM_USER0 from interrupt
> context.
So should I replace it by e.g. KM_IRQ0?
I'm not so familiar with these parts, and I couldn't find what these values
really mean.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: PS3 Storage Driver O_DIRECT issue
2007-07-13 12:27 ` PS3 Storage Driver O_DIRECT issue Olaf Hering
@ 2007-07-13 13:34 ` Geert Uytterhoeven
2007-07-17 16:40 ` Geoff Levand
2007-07-18 16:12 ` Geert Uytterhoeven
1 sibling, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 13:34 UTC (permalink / raw)
To: Olaf Hering
Cc: Paul Mackerras, Jens Axboe, James E.J. Bottomley,
Alessandro Rubini, linuxppc-dev, linux-kernel, linux-scsi
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1204 bytes --]
On Fri, 13 Jul 2007, Olaf Hering wrote:
> This driver (or the generic PS3 code) has appearently problems with
> O_DIRECT.
> glibc aborts parted because the malloc metadata get corrupted. While it
> is reproducible, the place where it crashes changes with every version
> of the debug attempt.
> I dont have a handle right now, all I know is that the metadata after a
> malloc area get overwritten with zeros.
>
>
> Can you have a look at this?
> parted /dev/ps3da
> print (a few times)
I can't seem to reproduce this with parted 1.7.1-5.1 (from Debian
etch/lenny/sid) and kernel 2.6.22-g77320894.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:25 ` Geert Uytterhoeven
@ 2007-07-13 13:34 ` James Bottomley
2007-07-13 13:45 ` Geert Uytterhoeven
2007-07-13 14:51 ` Jens Axboe
1 sibling, 1 reply; 46+ messages in thread
From: James Bottomley @ 2007-07-13 13:34 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, Paul Mackerras, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
On Fri, 2007-07-13 at 15:25 +0200, Geert Uytterhoeven wrote:
> kmap() just returns page_address() on ppc64, as there's no highmem.
> kunmap() is a no-op.
> So technically I could just use page_address() directly, but Christoph
> wanted
> me to keep the kmap()/kunmap() sequence because it's considered a good
> practice.
The point isn't what kmap and kunmap do ... it's the addresses they
return. By and large, a kernel virtual address for a page is different
from the user virtual address. If the cache is virtually indexed you
get different cache lines for the same page ... and that sets you up
with aliases you need to resolve. parisc is the same ... our
kmap/kunmap are nops as well, but our kernel virtual addresses are still
different from the user virtual ones.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:34 ` James Bottomley
@ 2007-07-13 13:45 ` Geert Uytterhoeven
2007-07-13 14:02 ` James Bottomley
0 siblings, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 13:45 UTC (permalink / raw)
To: James Bottomley
Cc: Jens Axboe, Paul Mackerras, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1711 bytes --]
On Fri, 13 Jul 2007, James Bottomley wrote:
> On Fri, 2007-07-13 at 15:25 +0200, Geert Uytterhoeven wrote:
> > kmap() just returns page_address() on ppc64, as there's no highmem.
> > kunmap() is a no-op.
>
> > So technically I could just use page_address() directly, but Christoph
> > wanted
> > me to keep the kmap()/kunmap() sequence because it's considered a good
> > practice.
>
> The point isn't what kmap and kunmap do ... it's the addresses they
> return. By and large, a kernel virtual address for a page is different
> from the user virtual address. If the cache is virtually indexed you
> get different cache lines for the same page ... and that sets you up
> with aliases you need to resolve. parisc is the same ... our
> kmap/kunmap are nops as well, but our kernel virtual addresses are still
> different from the user virtual ones.
IC.
- flush_kernel_dcache_page() is a no-op on ppc64
(ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
- For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
kmap/memcpy/kunmap sequence
So what should I do?
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:45 ` Geert Uytterhoeven
@ 2007-07-13 14:02 ` James Bottomley
2007-07-13 14:19 ` Arnd Bergmann
0 siblings, 1 reply; 46+ messages in thread
From: James Bottomley @ 2007-07-13 14:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, Paul Mackerras, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
On Fri, 2007-07-13 at 15:45 +0200, Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, James Bottomley wrote:
> > On Fri, 2007-07-13 at 15:25 +0200, Geert Uytterhoeven wrote:
> > > kmap() just returns page_address() on ppc64, as there's no highmem.
> > > kunmap() is a no-op.
> >
> > > So technically I could just use page_address() directly, but Christoph
> > > wanted
> > > me to keep the kmap()/kunmap() sequence because it's considered a good
> > > practice.
> >
> > The point isn't what kmap and kunmap do ... it's the addresses they
> > return. By and large, a kernel virtual address for a page is different
> > from the user virtual address. If the cache is virtually indexed you
> > get different cache lines for the same page ... and that sets you up
> > with aliases you need to resolve. parisc is the same ... our
> > kmap/kunmap are nops as well, but our kernel virtual addresses are still
> > different from the user virtual ones.
>
> IC.
>
> - flush_kernel_dcache_page() is a no-op on ppc64
> (ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
>
> - For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
> kmap/memcpy/kunmap sequence
>
> So what should I do?
Ask someone who knows the architecture ... Anton, Paulus or Benh ... I'm
fairly certain PPC is VIPT and will need some kind of alias
resolution ... perhaps its associative enough not to let the aliases be
a problem.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 14:02 ` James Bottomley
@ 2007-07-13 14:19 ` Arnd Bergmann
2007-07-13 15:10 ` Geert Uytterhoeven
2007-07-13 23:12 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 46+ messages in thread
From: Arnd Bergmann @ 2007-07-13 14:19 UTC (permalink / raw)
To: linuxppc-dev
Cc: James Bottomley, Geert Uytterhoeven, linux-scsi, linux-kernel,
Alessandro Rubini, Paul Mackerras, Jens Axboe
On Friday 13 July 2007, James Bottomley wrote:
>
> > IC.
> >
> > - flush_kernel_dcache_page() is a no-op on ppc64
> > (ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
> >
> > - For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
> > kmap/memcpy/kunmap sequence
> >
> > So what should I do?
>
> Ask someone who knows the architecture ... Anton, Paulus or Benh ... I'm
> fairly certain PPC is VIPT and will need some kind of alias
> resolution ... perhaps its associative enough not to let the aliases be
> a problem.
I'm pretty sure that no ppc64 machine needs alias resolution in the kernel,
although some are VIPT. Last time we discussed this, Segher explained it
to me, but I don't remember which way Cell does it. IIRC, it automatically
flushes cache lines that are accessed through aliases.
It's probably a good idea to have the flush_kernel_dcache_page() in there
anyway, if only to serve as an example for people that copy it into
architecture-independent drivers, same as what we do for the
k{,un}map_atomic() that is also not required on ppc64.
Arnd <><
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:25 ` Geert Uytterhoeven
2007-07-13 13:34 ` James Bottomley
@ 2007-07-13 14:51 ` Jens Axboe
1 sibling, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-13 14:51 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: James Bottomley, Paul Mackerras, Alessandro Rubini, linuxppc-dev,
linux-kernel, linux-scsi, Geoff Levand
On Fri, Jul 13 2007, Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, Jens Axboe wrote:
> > On Fri, Jul 13 2007, James Bottomley wrote:
> > > On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:
> > > > + kaddr = kmap_atomic(sgpnt->page, KM_USER0);
> > > > + if (!kaddr)
> > > > + return -1;
> > > > + len = sgpnt->length;
> > > > + if ((req_len + len) > buflen) {
> > > > + active = 0;
> > > > + len = buflen - req_len;
> > > > + }
> > > > + memcpy(kaddr + sgpnt->offset, buf + req_len,
> > > > len);
> > > > + kunmap_atomic(kaddr, KM_USER0);
> > >
> > > This isn't a SCSI objection, but this sequence appears several times in
> > > this driver. It's wrong for a non-PIPT architecture (and I believe the
> > > PS3 is VIPT) because you copy into the kernel alias for the page, which
> > > dirties the line in the cache of that alias (the user alias cache line
> > > was already invalidated). However, unless you flush the kernel alias to
> > > main memory, the user could read stale data. The way this is supposed
> > > to be done is to do a
> > >
> > > flush_kernel_dcache_page(kaddr)
> > >
> > > before doing the kunmap.
> > >
> > > Otherwise it looks OK from the SCSI point of view.
>
> kmap() just returns page_address() on ppc64, as there's no highmem.
> kunmap() is a no-op.
>
> So technically I could just use page_address() directly, but Christoph wanted
> me to keep the kmap()/kunmap() sequence because it's considered a good
> practice.
If you have the kmap sequence there, put the flush in as well. People
copy code, you know... Or put a big comment explaining why it isn't
needed.
> > Well, even worse is that fact that it's using KM_USER0 from interrupt
> > context.
>
> So should I replace it by e.g. KM_IRQ0?
> I'm not so familiar with these parts, and I couldn't find what these values
> really mean.
You corrupt data, using KM_USER0 from interrupt context. So it's a big
flaw right now. Use KM_IRQ0 for code where interrupts are always
disabled.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 14:19 ` Arnd Bergmann
@ 2007-07-13 15:10 ` Geert Uytterhoeven
2007-07-13 15:28 ` James Bottomley
2007-07-13 16:35 ` Jens Axboe
2007-07-13 23:12 ` Benjamin Herrenschmidt
1 sibling, 2 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 15:10 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linuxppc-dev, James Bottomley, linux-scsi, linux-kernel,
Alessandro Rubini, Paul Mackerras, Jens Axboe
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 2152 bytes --]
On Fri, 13 Jul 2007, Arnd Bergmann wrote:
> On Friday 13 July 2007, James Bottomley wrote:
> > > IC.
> > >
> > > - flush_kernel_dcache_page() is a no-op on ppc64
> > > (ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
> > >
> > > - For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
> > > kmap/memcpy/kunmap sequence
> > >
> > > So what should I do?
> >
> > Ask someone who knows the architecture ... Anton, Paulus or Benh ... I'm
> > fairly certain PPC is VIPT and will need some kind of alias
> > resolution ... perhaps its associative enough not to let the aliases be
> > a problem.
>
> I'm pretty sure that no ppc64 machine needs alias resolution in the kernel,
> although some are VIPT. Last time we discussed this, Segher explained it
> to me, but I don't remember which way Cell does it. IIRC, it automatically
> flushes cache lines that are accessed through aliases.
Thanks for confirming!
> It's probably a good idea to have the flush_kernel_dcache_page() in there
> anyway, if only to serve as an example for people that copy it into
> architecture-independent drivers, same as what we do for the
> k{,un}map_atomic() that is also not required on ppc64.
Now my next question: why should I add it, if currently no single driver in
mainline calls flush_kernel_dcache_page()?
`git grep' finds it in the following files only:
Documentation/cachetlb.txt
arch/parisc/kernel/cache.c
arch/parisc/kernel/pacache.S
include/asm-parisc/cacheflush.h
include/linux/highmem.h
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 15:10 ` Geert Uytterhoeven
@ 2007-07-13 15:28 ` James Bottomley
2007-07-13 20:13 ` Geert Uytterhoeven
2007-07-13 16:35 ` Jens Axboe
1 sibling, 1 reply; 46+ messages in thread
From: James Bottomley @ 2007-07-13 15:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, linuxppc-dev, linux-scsi, linux-kernel,
Alessandro Rubini, Paul Mackerras, Jens Axboe
On Fri, 2007-07-13 at 17:10 +0200, Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, Arnd Bergmann wrote:
> > On Friday 13 July 2007, James Bottomley wrote:
> > > > IC.
> > > >
> > > > - flush_kernel_dcache_page() is a no-op on ppc64
> > > > (ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
> > > >
> > > > - For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
> > > > kmap/memcpy/kunmap sequence
> > > >
> > > > So what should I do?
> > >
> > > Ask someone who knows the architecture ... Anton, Paulus or Benh ... I'm
> > > fairly certain PPC is VIPT and will need some kind of alias
> > > resolution ... perhaps its associative enough not to let the aliases be
> > > a problem.
> >
> > I'm pretty sure that no ppc64 machine needs alias resolution in the kernel,
> > although some are VIPT. Last time we discussed this, Segher explained it
> > to me, but I don't remember which way Cell does it. IIRC, it automatically
> > flushes cache lines that are accessed through aliases.
>
> Thanks for confirming!
>
> > It's probably a good idea to have the flush_kernel_dcache_page() in there
> > anyway, if only to serve as an example for people that copy it into
> > architecture-independent drivers, same as what we do for the
> > k{,un}map_atomic() that is also not required on ppc64.
>
> Now my next question: why should I add it, if currently no single driver in
> mainline calls flush_kernel_dcache_page()?
>
> `git grep' finds it in the following files only:
> Documentation/cachetlb.txt
> arch/parisc/kernel/cache.c
> arch/parisc/kernel/pacache.S
> include/asm-parisc/cacheflush.h
> include/linux/highmem.h
It's a recent addition to the API ... the previous way of doing it was
flush_dcache_page() but that's expensive and flushes all the user
aliases. Since you know in this case there are no current user aliases
and you only need to flush the kernel alias, flush_kernel_dcache_page()
was introduced as the cheaper alternative.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 15:10 ` Geert Uytterhoeven
2007-07-13 15:28 ` James Bottomley
@ 2007-07-13 16:35 ` Jens Axboe
1 sibling, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-13 16:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, linuxppc-dev, James Bottomley, linux-scsi,
linux-kernel, Alessandro Rubini, Paul Mackerras
On Fri, Jul 13 2007, Geert Uytterhoeven wrote:
> > It's probably a good idea to have the flush_kernel_dcache_page() in there
> > anyway, if only to serve as an example for people that copy it into
> > architecture-independent drivers, same as what we do for the
> > k{,un}map_atomic() that is also not required on ppc64.
>
> Now my next question: why should I add it, if currently no single driver in
> mainline calls flush_kernel_dcache_page()?
>
> `git grep' finds it in the following files only:
> Documentation/cachetlb.txt
> arch/parisc/kernel/cache.c
> arch/parisc/kernel/pacache.S
> include/asm-parisc/cacheflush.h
> include/linux/highmem.h
Not many drivers fiddle around with stuff like this, it's usually hidden
behind the dma api or in helpers.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 15:28 ` James Bottomley
@ 2007-07-13 20:13 ` Geert Uytterhoeven
2007-07-16 11:43 ` Geert Uytterhoeven
0 siblings, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-13 20:13 UTC (permalink / raw)
To: James Bottomley
Cc: Arnd Bergmann, linuxppc-dev, linux-scsi, linux-kernel,
Alessandro Rubini, Paul Mackerras, Jens Axboe
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2859 bytes --]
On Fri, 13 Jul 2007, James Bottomley wrote:
> On Fri, 2007-07-13 at 17:10 +0200, Geert Uytterhoeven wrote:
> > On Fri, 13 Jul 2007, Arnd Bergmann wrote:
> > > On Friday 13 July 2007, James Bottomley wrote:
> > > > > IC.
> > > > >
> > > > > - flush_kernel_dcache_page() is a no-op on ppc64
> > > > > (ARCH_HAS_FLUSH_KERNEL_DCACHE_PAGE is defined on parisc only).
> > > > >
> > > > > - For reference, drivers/scsi/ipr.c (another ppc64 driver) just uses a plain
> > > > > kmap/memcpy/kunmap sequence
> > > > >
> > > > > So what should I do?
> > > >
> > > > Ask someone who knows the architecture ... Anton, Paulus or Benh ... I'm
> > > > fairly certain PPC is VIPT and will need some kind of alias
> > > > resolution ... perhaps its associative enough not to let the aliases be
> > > > a problem.
> > >
> > > I'm pretty sure that no ppc64 machine needs alias resolution in the kernel,
> > > although some are VIPT. Last time we discussed this, Segher explained it
> > > to me, but I don't remember which way Cell does it. IIRC, it automatically
> > > flushes cache lines that are accessed through aliases.
> >
> > Thanks for confirming!
> >
> > > It's probably a good idea to have the flush_kernel_dcache_page() in there
> > > anyway, if only to serve as an example for people that copy it into
> > > architecture-independent drivers, same as what we do for the
> > > k{,un}map_atomic() that is also not required on ppc64.
> >
> > Now my next question: why should I add it, if currently no single driver in
> > mainline calls flush_kernel_dcache_page()?
> >
> > `git grep' finds it in the following files only:
> > Documentation/cachetlb.txt
> > arch/parisc/kernel/cache.c
> > arch/parisc/kernel/pacache.S
> > include/asm-parisc/cacheflush.h
> > include/linux/highmem.h
>
> It's a recent addition to the API ... the previous way of doing it was
> flush_dcache_page() but that's expensive and flushes all the user
> aliases. Since you know in this case there are no current user aliases
> and you only need to flush the kernel alias, flush_kernel_dcache_page()
> was introduced as the cheaper alternative.
Ah, that explains it. flush_dcache_page() is used in some drivers.
I'll update my patches. Thanks for the comments!
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 13:02 ` James Bottomley
2007-07-13 13:05 ` Jens Axboe
@ 2007-07-13 23:06 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-13 23:06 UTC (permalink / raw)
To: James Bottomley
Cc: Geert Uytterhoeven, Jens Axboe, linux-scsi, linux-kernel,
Alessandro Rubini, linuxppc-dev, Paul Mackerras
On Fri, 2007-07-13 at 09:02 -0400, James Bottomley wrote:
> On Wed, 2007-07-04 at 15:22 +0200, Geert Uytterhoeven wrote:
> > + kaddr = kmap_atomic(sgpnt->page, KM_USER0);
> > + if (!kaddr)
> > + return -1;
> > + len = sgpnt->length;
> > + if ((req_len + len) > buflen) {
> > + active = 0;
> > + len = buflen - req_len;
> > + }
> > + memcpy(kaddr + sgpnt->offset, buf + req_len,
> > len);
> > + kunmap_atomic(kaddr, KM_USER0);
>
> This isn't a SCSI objection, but this sequence appears several times in
> this driver. It's wrong for a non-PIPT architecture (and I believe the
> PS3 is VIPT) because you copy into the kernel alias for the page, which
> dirties the line in the cache of that alias (the user alias cache line
> was already invalidated). However, unless you flush the kernel alias to
> main memory, the user could read stale data. The way this is supposed
> to be done is to do a
Nah, we have no cache aliasing on ppc, at least not that matter here and
not on cell.
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 14:19 ` Arnd Bergmann
2007-07-13 15:10 ` Geert Uytterhoeven
@ 2007-07-13 23:12 ` Benjamin Herrenschmidt
2007-07-16 12:08 ` Segher Boessenkool
1 sibling, 1 reply; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-13 23:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linuxppc-dev, James Bottomley, linux-scsi, linux-kernel,
Alessandro Rubini, Paul Mackerras, Jens Axboe,
Geert Uytterhoeven
On Fri, 2007-07-13 at 16:19 +0200, Arnd Bergmann wrote:
> I'm pretty sure that no ppc64 machine needs alias resolution in the kernel,
> although some are VIPT. Last time we discussed this, Segher explained it
> to me, but I don't remember which way Cell does it. IIRC, it automatically
> flushes cache lines that are accessed through aliases.
Ah yes, I remember reading about this automatic flushing thing. I don't
know how the caches actually work on most of our PPC's, but the fact is,
we don't have aliasing issues, so I can safely ignore it for a bit
longer :-)
There are some aliasing issues with the instruction cache specifically
on some 4xx models but that's irrelevant to this discussion (and I think
we handle them elsewhere anyway).
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 20:13 ` Geert Uytterhoeven
@ 2007-07-16 11:43 ` Geert Uytterhoeven
2007-07-16 12:16 ` Jens Axboe
0 siblings, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-16 11:43 UTC (permalink / raw)
To: James Bottomley, Jens Axboe
Cc: Arnd Bergmann, linux-scsi, Linux Kernel Development,
Alessandro Rubini, Linux/PPC Development, Paul Mackerras
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3186 bytes --]
On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> Ah, that explains it. flush_dcache_page() is used in some drivers.
> I'll update my patches. Thanks for the comments!
Does this look OK?
- Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
interrupt handler, from .request_fn (ps3disk), or from .queuecommand
(ps3rom))
- Add a call to flush_kernel_dcache_page() in routines that write to buffers
If this is OK, I'll fold it into my original patch series and will resend.
Thanks!
---
drivers/block/ps3disk.c | 5 +++--
drivers/scsi/ps3rom.c | 9 +++++----
2 files changed, 8 insertions(+), 6 deletions(-)
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -109,13 +109,14 @@ static void ps3disk_scatter_gather(struc
bio_sectors(bio), sector);
bio_for_each_segment(bvec, bio, j) {
size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
- buf = __bio_kmap_atomic(bio, j, KM_USER0);
+ buf = __bio_kmap_atomic(bio, j, KM_IRQ0);
if (gather)
memcpy(dev->bounce_buf+offset, buf, size);
else
memcpy(buf, dev->bounce_buf+offset, size);
offset += size;
- __bio_kunmap_atomic(bio, KM_USER0);
+ flush_kernel_dcache_page(bio_iovec_idx(bio, j)->bv_page);
+ __bio_kunmap_atomic(bio, KM_IRQ0);
}
sectors += bio_sectors(bio);
i++;
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -112,7 +112,7 @@ static int fill_from_dev_buffer(struct s
active = 1;
for (k = 0, req_len = 0, act_len = 0; k < cmd->use_sg; ++k, ++sgpnt) {
if (active) {
- kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+ kaddr = kmap_atomic(sgpnt->page, KM_IRQ0);
if (!kaddr)
return -1;
len = sgpnt->length;
@@ -121,7 +121,8 @@ static int fill_from_dev_buffer(struct s
len = buflen - req_len;
}
memcpy(kaddr + sgpnt->offset, buf + req_len, len);
- kunmap_atomic(kaddr, KM_USER0);
+ flush_kernel_dcache_page(sgpnt->page);
+ kunmap_atomic(kaddr, KM_IRQ0);
act_len += len;
}
req_len += sgpnt->length;
@@ -149,7 +150,7 @@ static int fetch_to_dev_buffer(struct sc
sgpnt = cmd->request_buffer;
for (k = 0, req_len = 0, fin = 0; k < cmd->use_sg; ++k, ++sgpnt) {
- kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+ kaddr = kmap_atomic(sgpnt->page, KM_IRQ0);
if (!kaddr)
return -1;
len = sgpnt->length;
@@ -158,7 +159,7 @@ static int fetch_to_dev_buffer(struct sc
fin = 1;
}
memcpy(buf + req_len, kaddr + sgpnt->offset, len);
- kunmap_atomic(kaddr, KM_USER0);
+ kunmap_atomic(kaddr, KM_IRQ0);
if (fin)
return req_len + len;
req_len += sgpnt->length;
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-13 23:12 ` Benjamin Herrenschmidt
@ 2007-07-16 12:08 ` Segher Boessenkool
0 siblings, 0 replies; 46+ messages in thread
From: Segher Boessenkool @ 2007-07-16 12:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Arnd Bergmann, James Bottomley, linux-scsi, linux-kernel,
Alessandro Rubini, linuxppc-dev, Paul Mackerras, Jens Axboe,
Geert Uytterhoeven
>> I'm pretty sure that no ppc64 machine needs alias resolution in
>> the kernel,
>> although some are VIPT. Last time we discussed this, Segher
>> explained it
>> to me, but I don't remember which way Cell does it. IIRC, it
>> automatically
>> flushes cache lines that are accessed through aliases.
>
> Ah yes, I remember reading about this automatic flushing thing. I
> don't
> know how the caches actually work on most of our PPC's, but the
> fact is,
> we don't have aliasing issues, so I can safely ignore it for a bit
> longer :-)
That is the very short version of the story, yes: some
PowerPC implementations are VIPT physically, but there
are no aliasing issues we have to worry about.
Anyone interested in how this works, can download the
PPC970 UM (or 970FX or 970MP); it has a very detailed
explanation of all this. Cell might be slightly different
but the base idea is the same.
Segher
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 11:43 ` Geert Uytterhoeven
@ 2007-07-16 12:16 ` Jens Axboe
2007-07-16 12:59 ` Geert Uytterhoeven
2007-07-16 13:47 ` James Bottomley
0 siblings, 2 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-16 12:16 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > I'll update my patches. Thanks for the comments!
>
> Does this look OK?
> - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> (ps3rom))
That looks good.
> - Add a call to flush_kernel_dcache_page() in routines that write to buffers
Hmm, I would have thought a flush_dcache_page() would be more
appropriate, the backing could be page cache pages.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 12:16 ` Jens Axboe
@ 2007-07-16 12:59 ` Geert Uytterhoeven
2007-07-16 13:04 ` Jens Axboe
2007-07-16 13:24 ` Geert Uytterhoeven
2007-07-16 13:47 ` James Bottomley
1 sibling, 2 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-16 12:59 UTC (permalink / raw)
To: Jens Axboe
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1342 bytes --]
On Mon, 16 Jul 2007, Jens Axboe wrote:
> On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > I'll update my patches. Thanks for the comments!
> >
> > Does this look OK?
> > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > (ps3rom))
>
> That looks good.
>
> > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
>
> Hmm, I would have thought a flush_dcache_page() would be more
> appropriate, the backing could be page cache pages.
OK, I'll change it to flush_dcache_page().
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 12:59 ` Geert Uytterhoeven
@ 2007-07-16 13:04 ` Jens Axboe
2007-07-16 13:24 ` Geert Uytterhoeven
1 sibling, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-16 13:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> On Mon, 16 Jul 2007, Jens Axboe wrote:
> > On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > > I'll update my patches. Thanks for the comments!
> > >
> > > Does this look OK?
> > > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > > (ps3rom))
> >
> > That looks good.
> >
> > > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
> >
> > Hmm, I would have thought a flush_dcache_page() would be more
> > appropriate, the backing could be page cache pages.
>
> OK, I'll change it to flush_dcache_page().
Then you may add my acked-by as well, if you want.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 12:59 ` Geert Uytterhoeven
2007-07-16 13:04 ` Jens Axboe
@ 2007-07-16 13:24 ` Geert Uytterhoeven
2007-07-16 13:33 ` Jens Axboe
2007-07-16 21:38 ` Benjamin Herrenschmidt
1 sibling, 2 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-16 13:24 UTC (permalink / raw)
To: Jens Axboe
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1687 bytes --]
On Mon, 16 Jul 2007, Geert Uytterhoeven wrote:
> On Mon, 16 Jul 2007, Jens Axboe wrote:
> > On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > > I'll update my patches. Thanks for the comments!
> > >
> > > Does this look OK?
> > > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > > (ps3rom))
> >
> > That looks good.
> >
> > > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
> >
> > Hmm, I would have thought a flush_dcache_page() would be more
> > appropriate, the backing could be page cache pages.
>
> OK, I'll change it to flush_dcache_page().
Upon closer look, while flush_kernel_dcache_page() is a no-op on ppc64,
flush_dcache_page() isn't. So I'd prefer to not call it if not really needed.
And according to James, flush_kernel_dcache_page() should be sufficient...
So I'm getting puzzled again...
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 13:24 ` Geert Uytterhoeven
@ 2007-07-16 13:33 ` Jens Axboe
2007-07-16 21:38 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-16 13:33 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> On Mon, 16 Jul 2007, Geert Uytterhoeven wrote:
> > On Mon, 16 Jul 2007, Jens Axboe wrote:
> > > On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > > > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > > > I'll update my patches. Thanks for the comments!
> > > >
> > > > Does this look OK?
> > > > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > > > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > > > (ps3rom))
> > >
> > > That looks good.
> > >
> > > > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
> > >
> > > Hmm, I would have thought a flush_dcache_page() would be more
> > > appropriate, the backing could be page cache pages.
> >
> > OK, I'll change it to flush_dcache_page().
>
> Upon closer look, while flush_kernel_dcache_page() is a no-op on ppc64,
> flush_dcache_page() isn't. So I'd prefer to not call it if not really needed.
>
> And according to James, flush_kernel_dcache_page() should be sufficient...
>
> So I'm getting puzzled again...
I'll let James expand on why he thinks flush_kernel_dcache_page() should
be sufficient. If the backing is always user memory from
get_user_pages(), then the flush_kernel_dcache_page() looks sufficient.
For the normal IO paths, it could be that or page cache pages too for
instance.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 12:16 ` Jens Axboe
2007-07-16 12:59 ` Geert Uytterhoeven
@ 2007-07-16 13:47 ` James Bottomley
2007-07-16 14:38 ` Jens Axboe
` (2 more replies)
1 sibling, 3 replies; 46+ messages in thread
From: James Bottomley @ 2007-07-16 13:47 UTC (permalink / raw)
To: Jens Axboe
Cc: Geert Uytterhoeven, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
On Mon, 2007-07-16 at 14:16 +0200, Jens Axboe wrote:
> On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > I'll update my patches. Thanks for the comments!
> >
> > Does this look OK?
> > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > (ps3rom))
>
> That looks good.
>
> > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
>
> Hmm, I would have thought a flush_dcache_page() would be more
> appropriate, the backing could be page cache pages.
No ... that was the point of flush_kernel_dcache_page(). The page in
question is page cache backed and contains user mappings. However, the
block layer has already done a flush_dcache_page() in get_user_pages()
and the user shouldn't be touching memory under I/O (unless they want
self induced aliasing problems) so we're free to assume all the user
cachelines are purged, hence all we have to do is flush the kernel alias
to bring the page up to date and make the users see it correctly.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 13:47 ` James Bottomley
@ 2007-07-16 14:38 ` Jens Axboe
2007-07-16 14:59 ` Geert Uytterhoeven
2007-07-16 21:40 ` Benjamin Herrenschmidt
2007-07-16 21:49 ` Benjamin Herrenschmidt
2 siblings, 1 reply; 46+ messages in thread
From: Jens Axboe @ 2007-07-16 14:38 UTC (permalink / raw)
To: James Bottomley
Cc: Geert Uytterhoeven, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
On Mon, Jul 16 2007, James Bottomley wrote:
> On Mon, 2007-07-16 at 14:16 +0200, Jens Axboe wrote:
> > On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > > I'll update my patches. Thanks for the comments!
> > >
> > > Does this look OK?
> > > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > > (ps3rom))
> >
> > That looks good.
> >
> > > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
> >
> > Hmm, I would have thought a flush_dcache_page() would be more
> > appropriate, the backing could be page cache pages.
>
> No ... that was the point of flush_kernel_dcache_page(). The page in
> question is page cache backed and contains user mappings. However, the
> block layer has already done a flush_dcache_page() in get_user_pages()
> and the user shouldn't be touching memory under I/O (unless they want
> self induced aliasing problems) so we're free to assume all the user
> cachelines are purged, hence all we have to do is flush the kernel alias
> to bring the page up to date and make the users see it correctly.
Oh indeed, I missed the flush_dcache_page() in get_user_pages().
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 14:38 ` Jens Axboe
@ 2007-07-16 14:59 ` Geert Uytterhoeven
0 siblings, 0 replies; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-16 14:59 UTC (permalink / raw)
To: Jens Axboe
Cc: James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2132 bytes --]
On Mon, 16 Jul 2007, Jens Axboe wrote:
> On Mon, Jul 16 2007, James Bottomley wrote:
> > On Mon, 2007-07-16 at 14:16 +0200, Jens Axboe wrote:
> > > On Mon, Jul 16 2007, Geert Uytterhoeven wrote:
> > > > On Fri, 13 Jul 2007, Geert Uytterhoeven wrote:
> > > > > Ah, that explains it. flush_dcache_page() is used in some drivers.
> > > > > I'll update my patches. Thanks for the comments!
> > > >
> > > > Does this look OK?
> > > > - Replaced KM_USER0 by KM_IRQ0 (all routines are either called from an
> > > > interrupt handler, from .request_fn (ps3disk), or from .queuecommand
> > > > (ps3rom))
> > >
> > > That looks good.
> > >
> > > > - Add a call to flush_kernel_dcache_page() in routines that write to buffers
> > >
> > > Hmm, I would have thought a flush_dcache_page() would be more
> > > appropriate, the backing could be page cache pages.
> >
> > No ... that was the point of flush_kernel_dcache_page(). The page in
> > question is page cache backed and contains user mappings. However, the
> > block layer has already done a flush_dcache_page() in get_user_pages()
> > and the user shouldn't be touching memory under I/O (unless they want
> > self induced aliasing problems) so we're free to assume all the user
> > cachelines are purged, hence all we have to do is flush the kernel alias
> > to bring the page up to date and make the users see it correctly.
>
> Oh indeed, I missed the flush_dcache_page() in get_user_pages().
Good, thanks for reaching a consensus, so I can update my patch series.
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 13:24 ` Geert Uytterhoeven
2007-07-16 13:33 ` Jens Axboe
@ 2007-07-16 21:38 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-16 21:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Jens Axboe, James Bottomley, Arnd Bergmann, linux-scsi,
Linux Kernel Development, Alessandro Rubini,
Linux/PPC Development, Paul Mackerras
> Upon closer look, while flush_kernel_dcache_page() is a no-op on ppc64,
> flush_dcache_page() isn't. So I'd prefer to not call it if not really needed.
>
> And according to James, flush_kernel_dcache_page() should be sufficient...
>
> So I'm getting puzzled again...
flush_dcache_page() handles icache vs. dcache coherency by clearing the
PG_arch_1 bit in the struct page that indicates that the page is cache
clean.
You -must- call it if you're going to use any form of CPU access to
write to the page (basically dirtying the data cache) and that page can
be ever mapped into user space and executed from.
I don't know what flush_kernel_dcache_page() does and if it needs a
similar treatement, it's a new interface, so maybe Jens and or James can
tell me more about it..
Cheers,
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 13:47 ` James Bottomley
2007-07-16 14:38 ` Jens Axboe
@ 2007-07-16 21:40 ` Benjamin Herrenschmidt
2007-07-16 21:49 ` Benjamin Herrenschmidt
2 siblings, 0 replies; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-16 21:40 UTC (permalink / raw)
To: James Bottomley
Cc: Jens Axboe, Arnd Bergmann, linux-scsi, Linux Kernel Development,
Alessandro Rubini, Linux/PPC Development, Paul Mackerras,
Geert Uytterhoeven
On Mon, 2007-07-16 at 08:47 -0500, James Bottomley wrote:
>
> No ... that was the point of flush_kernel_dcache_page(). The page in
> question is page cache backed and contains user mappings. However,
> the
> block layer has already done a flush_dcache_page() in get_user_pages()
> and the user shouldn't be touching memory under I/O (unless they want
> self induced aliasing problems) so we're free to assume all the user
> cachelines are purged, hence all we have to do is flush the kernel
> alias
> to bring the page up to date and make the users see it correctly.
Ok. In our case the problem is not aliases though, it's the coherency
between instruction and data caches for pages that may be executed from
(such as swapped out text pages).
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 13:47 ` James Bottomley
2007-07-16 14:38 ` Jens Axboe
2007-07-16 21:40 ` Benjamin Herrenschmidt
@ 2007-07-16 21:49 ` Benjamin Herrenschmidt
2007-07-16 22:03 ` James Bottomley
2 siblings, 1 reply; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-16 21:49 UTC (permalink / raw)
To: James Bottomley
Cc: Jens Axboe, Arnd Bergmann, linux-scsi, Linux Kernel Development,
Alessandro Rubini, Linux/PPC Development, Paul Mackerras,
Geert Uytterhoeven
> No ... that was the point of flush_kernel_dcache_page(). The page in
> question is page cache backed and contains user mappings. However, the
> block layer has already done a flush_dcache_page() in get_user_pages()
> and the user shouldn't be touching memory under I/O (unless they want
> self induced aliasing problems) so we're free to assume all the user
> cachelines are purged, hence all we have to do is flush the kernel alias
> to bring the page up to date and make the users see it correctly.
The block layer will have done that even in the swap-out path ? (Just
asking... I'm not very familiar with the block layer)
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 21:49 ` Benjamin Herrenschmidt
@ 2007-07-16 22:03 ` James Bottomley
2007-07-16 22:12 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 46+ messages in thread
From: James Bottomley @ 2007-07-16 22:03 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Jens Axboe, Arnd Bergmann, linux-scsi, Linux Kernel Development,
Alessandro Rubini, Linux/PPC Development, Paul Mackerras,
Geert Uytterhoeven
On Tue, 2007-07-17 at 07:49 +1000, Benjamin Herrenschmidt wrote:
> > No ... that was the point of flush_kernel_dcache_page(). The page in
> > question is page cache backed and contains user mappings. However, the
> > block layer has already done a flush_dcache_page() in get_user_pages()
> > and the user shouldn't be touching memory under I/O (unless they want
> > self induced aliasing problems) so we're free to assume all the user
> > cachelines are purged, hence all we have to do is flush the kernel alias
> > to bring the page up to date and make the users see it correctly.
>
> The block layer will have done that even in the swap-out path ? (Just
> asking... I'm not very familiar with the block layer)
Er ... not really, this is the I/O path for user initiated I/O. The
page out path, by definition, can't have any extant user mappings. For
page out, the relevant page is flushed before its mapping is detached,
and then it can be paged to the backing store (or for anonymous pages to
the swap device) when no mappings remain.
James
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [patch 5/6] ps3: BD/DVD/CD-ROM Storage Driver
2007-07-16 22:03 ` James Bottomley
@ 2007-07-16 22:12 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 46+ messages in thread
From: Benjamin Herrenschmidt @ 2007-07-16 22:12 UTC (permalink / raw)
To: James Bottomley
Cc: Jens Axboe, Arnd Bergmann, linux-scsi, Linux Kernel Development,
Alessandro Rubini, Linux/PPC Development, Paul Mackerras,
Geert Uytterhoeven
On Mon, 2007-07-16 at 17:03 -0500, James Bottomley wrote:
> On Tue, 2007-07-17 at 07:49 +1000, Benjamin Herrenschmidt wrote:
> > > No ... that was the point of flush_kernel_dcache_page(). The page in
> > > question is page cache backed and contains user mappings. However, the
> > > block layer has already done a flush_dcache_page() in get_user_pages()
> > > and the user shouldn't be touching memory under I/O (unless they want
> > > self induced aliasing problems) so we're free to assume all the user
> > > cachelines are purged, hence all we have to do is flush the kernel alias
> > > to bring the page up to date and make the users see it correctly.
> >
> > The block layer will have done that even in the swap-out path ? (Just
> > asking... I'm not very familiar with the block layer)
>
> Er ... not really, this is the I/O path for user initiated I/O. The
> page out path, by definition, can't have any extant user mappings. For
> page out, the relevant page is flushed before its mapping is detached,
> and then it can be paged to the backing store (or for anonymous pages to
> the swap device) when no mappings remain.
Ok, thanks.
Ben.
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: PS3 Storage Driver O_DIRECT issue
2007-07-13 13:34 ` Geert Uytterhoeven
@ 2007-07-17 16:40 ` Geoff Levand
0 siblings, 0 replies; 46+ messages in thread
From: Geoff Levand @ 2007-07-17 16:40 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Olaf Hering, Jens Axboe, James E.J. Bottomley, linux-scsi,
Alessandro Rubini, linux-kernel, linuxppc-dev, Paul Mackerras
Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, Olaf Hering wrote:
>> This driver (or the generic PS3 code) has appearently problems with
>> O_DIRECT.
>> glibc aborts parted because the malloc metadata get corrupted. While it
>> is reproducible, the place where it crashes changes with every version
>> of the debug attempt.
>> I dont have a handle right now, all I know is that the metadata after a
>> malloc area get overwritten with zeros.
>>
>>
>> Can you have a look at this?
>> parted /dev/ps3da
>> print (a few times)
>
> I can't seem to reproduce this with parted 1.7.1-5.1 (from Debian
> etch/lenny/sid) and kernel 2.6.22-g77320894.
Hi.
I found this happens on Fedora 7:
[root@ps3-nfs ~]# uname -a
Linux ps3-nfs 2.6.22-ps3-linux-dev-g4d898766-dirty #1 SMP Wed Jul 11 13:29:46 PDT 2007 ppc64 ppc64 ppc64 GNU/Linux
[root@ps3-nfs ~]# parted --version
parted (GNU parted) 1.8.6
Here is the error message from parted:
Command History:
print
Error: SEGV_MAPERR (Address not mapped to object)
Backtrace has 20 calls on stack:
20: /usr/lib/libparted-1.8.so.6(ped_assert+0xb0) [0xfb7ea50]
19: parted [0x1000c6dc]
18: [0x100350]
17: [(nil)]
16: /lib/libc.so.6 [0xfdcfe64]
15: /lib/libc.so.6 [0xfdd0b34]
14: /lib/libc.so.6(__libc_memalign+0xec) [0xfdd1e1c]
13: /lib/libc.so.6(posix_memalign+0xbc) [0xfdd207c]
12: /usr/lib/libparted-1.8.so.6 [0xfb8f42c]
11: /usr/lib/libparted-1.8.so.6(ped_device_read+0x164) [0xfb7f5f4]
10: /usr/lib/libparted-1.8.so.6(ped_geometry_read+0x16c) [0xfb89a5c]
9: /usr/lib/libparted-1.8.so.6 [0xfba739c]
8: /usr/lib/libparted-1.8.so.6(ped_file_system_probe_specific+0x104) [0xfb80d04]
7: /usr/lib/libparted-1.8.so.6(ped_file_system_probe+0xec) [0xfb8134c]
6: /usr/lib/libparted-1.8.so.6 [0xfbbcc38]
5: /usr/lib/libparted-1.8.so.6 [0xfbbcfb4]
4: /usr/lib/libparted-1.8.so.6(ped_disk_new+0xc0) [0xfb88bf0]
3: parted [0x10006e00]
2: parted(command_run+0x1c) [0x10004d8c]
1: parted(interactive_mode+0x134) [0x1000e4b4]
Aborted
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: PS3 Storage Driver O_DIRECT issue
2007-07-13 12:27 ` PS3 Storage Driver O_DIRECT issue Olaf Hering
2007-07-13 13:34 ` Geert Uytterhoeven
@ 2007-07-18 16:12 ` Geert Uytterhoeven
2007-07-18 17:57 ` Jens Axboe
1 sibling, 1 reply; 46+ messages in thread
From: Geert Uytterhoeven @ 2007-07-18 16:12 UTC (permalink / raw)
To: Olaf Hering
Cc: Paul Mackerras, Jens Axboe, James E.J. Bottomley,
Alessandro Rubini, Geoff Levand, Linux/PPC Development,
Linux Kernel Development, linux-scsi
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1917 bytes --]
On Fri, 13 Jul 2007, Olaf Hering wrote:
> This driver (or the generic PS3 code) has appearently problems with
> O_DIRECT.
> glibc aborts parted because the malloc metadata get corrupted. While it
> is reproducible, the place where it crashes changes with every version
> of the debug attempt.
> I dont have a handle right now, all I know is that the metadata after a
> malloc area get overwritten with zeros.
>
>
> Can you have a look at this?
> parted /dev/ps3da
> print (a few times)
I could reproduce it with parted 1.8.0 and later.
The patch below fixes it (tested with 1.8.0, 1.8.2 (FC6), 1.9.0 (git)).
Apparently sometimes bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE != bvec->bv_len
when using O_DIRECT. Is that true (or a bug?)?
If it's OK, I'll fold this patch into the main ps3disk patch.
---
drivers/block/ps3disk.c | 2 +-
1 files changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -108,7 +108,7 @@ static void ps3disk_scatter_gather(struc
__func__, __LINE__, i, bio_segments(bio),
bio_sectors(bio), sector);
bio_for_each_segment(bvec, bio, j) {
- size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
+ size = bvec->bv_len;
buf = __bio_kmap_atomic(bio, j, KM_IRQ0);
if (gather)
memcpy(dev->bounce_buf+offset, buf, size);
With kind regards,
Geert Uytterhoeven
Software Architect
Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/
Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619
^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: PS3 Storage Driver O_DIRECT issue
2007-07-18 16:12 ` Geert Uytterhoeven
@ 2007-07-18 17:57 ` Jens Axboe
0 siblings, 0 replies; 46+ messages in thread
From: Jens Axboe @ 2007-07-18 17:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Olaf Hering, Paul Mackerras, James E.J. Bottomley,
Alessandro Rubini, Geoff Levand, Linux/PPC Development,
Linux Kernel Development, linux-scsi
On Wed, Jul 18 2007, Geert Uytterhoeven wrote:
> On Fri, 13 Jul 2007, Olaf Hering wrote:
> > This driver (or the generic PS3 code) has appearently problems with
> > O_DIRECT.
> > glibc aborts parted because the malloc metadata get corrupted. While it
> > is reproducible, the place where it crashes changes with every version
> > of the debug attempt.
> > I dont have a handle right now, all I know is that the metadata after a
> > malloc area get overwritten with zeros.
> >
> >
> > Can you have a look at this?
> > parted /dev/ps3da
> > print (a few times)
>
> I could reproduce it with parted 1.8.0 and later.
>
> The patch below fixes it (tested with 1.8.0, 1.8.2 (FC6), 1.9.0 (git)).
>
> Apparently sometimes bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE != bvec->bv_len
> when using O_DIRECT. Is that true (or a bug?)?
>
> If it's OK, I'll fold this patch into the main ps3disk patch.
The bug is in your code - you iterate the bio segments, but always
reference the current one. Which of course makes no sense, you may as
well just kill the loop in that case, the code would be the same.
--
Jens Axboe
^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2007-07-18 17:58 UTC | newest]
Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-04 13:22 [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-04 13:22 ` [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver Geert Uytterhoeven
2007-07-04 13:22 ` [patch 2/6] ps3: Storage Driver Core Geert Uytterhoeven
2007-07-04 13:22 ` [patch 3/6] ps3: Storage device registration routines Geert Uytterhoeven
2007-07-04 13:22 ` [patch 4/6] ps3: Disk Storage Driver Geert Uytterhoeven
2007-07-04 13:22 ` [patch 5/6] ps3: BD/DVD/CD-ROM " Geert Uytterhoeven
2007-07-13 13:02 ` James Bottomley
2007-07-13 13:05 ` Jens Axboe
2007-07-13 13:25 ` Geert Uytterhoeven
2007-07-13 13:34 ` James Bottomley
2007-07-13 13:45 ` Geert Uytterhoeven
2007-07-13 14:02 ` James Bottomley
2007-07-13 14:19 ` Arnd Bergmann
2007-07-13 15:10 ` Geert Uytterhoeven
2007-07-13 15:28 ` James Bottomley
2007-07-13 20:13 ` Geert Uytterhoeven
2007-07-16 11:43 ` Geert Uytterhoeven
2007-07-16 12:16 ` Jens Axboe
2007-07-16 12:59 ` Geert Uytterhoeven
2007-07-16 13:04 ` Jens Axboe
2007-07-16 13:24 ` Geert Uytterhoeven
2007-07-16 13:33 ` Jens Axboe
2007-07-16 21:38 ` Benjamin Herrenschmidt
2007-07-16 13:47 ` James Bottomley
2007-07-16 14:38 ` Jens Axboe
2007-07-16 14:59 ` Geert Uytterhoeven
2007-07-16 21:40 ` Benjamin Herrenschmidt
2007-07-16 21:49 ` Benjamin Herrenschmidt
2007-07-16 22:03 ` James Bottomley
2007-07-16 22:12 ` Benjamin Herrenschmidt
2007-07-13 16:35 ` Jens Axboe
2007-07-13 23:12 ` Benjamin Herrenschmidt
2007-07-16 12:08 ` Segher Boessenkool
2007-07-13 14:51 ` Jens Axboe
2007-07-13 23:06 ` Benjamin Herrenschmidt
2007-07-04 13:22 ` [patch 6/6] ps3: FLASH ROM " Geert Uytterhoeven
2007-07-10 7:45 ` [patch 0/6] PS3 Storage Drivers for 2.6.23, take 4 Geert Uytterhoeven
2007-07-10 7:55 ` Jens Axboe
2007-07-10 8:01 ` Paul Mackerras
2007-07-10 8:32 ` Jens Axboe
2007-07-13 9:31 ` Geert Uytterhoeven
2007-07-13 12:27 ` PS3 Storage Driver O_DIRECT issue Olaf Hering
2007-07-13 13:34 ` Geert Uytterhoeven
2007-07-17 16:40 ` Geoff Levand
2007-07-18 16:12 ` Geert Uytterhoeven
2007-07-18 17:57 ` Jens Axboe
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).