| Message ID | alpine.DEB.2.00.1104071816270.10238@peruna (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi John, On Thu, Apr 7, 2011 at 8:18 PM, John Calixto <john.calixto@modsystems.com> wrote: > + /* data.flags must already be set before doing this. */ > + mmc_set_data_timeout(&data, card); > + /* Allow overriding the timeout_ns for empirical tuning. */ > + if (idata->ic.force_timeout_ns) > + data.timeout_ns = idata->ic.force_timeout_ns; > + Are there any R1B ACMDs? > + mmc_wait_for_req(card->host, &mrq); > + > + if (cmd.error) { > + dev_err(mmc_dev(card->host), "%s: cmd error %d\n", > + __func__, cmd.error); > + err = cmd.error; > + goto acmd_rel_host; > + } > + if (data.error) { > + dev_err(mmc_dev(card->host), "%s: data error %d\n", > + __func__, data.error); > + err = data.error; > + goto acmd_rel_host; > + } > + > + /* > + * According to the SD specs, some commands require a delay after > + * issuing the command. > + */ > + if (idata->ic.postsleep_us) > + udelay(idata->ic.postsleep_us); > + How long can this possibly be? Does usleep make any sense here? > +struct mmc_ioc_cmd { > + int write_flag; /* implies direction of data. true = write, false = read */ > + __u32 opcode; > + __u32 arg; > + __u32 response[4]; /* CMD response */ > + unsigned int flags; > + unsigned int blksz; > + unsigned int blocks; I'm not sure I understand why blksz is passed. If you needed to set the block size (via CMD16) prior to some command (like MMC password protection) I could kind of get it, but then all I see the blk size getting used for is calculating the data amount in bytes, so you're not doing that. So why not just infer it from the card queue? A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 8, 2011 at 3:22 AM, Andrei Warkentin <andreiw@motorola.com> wrote: >> + >> + /* >> + * According to the SD specs, some commands require a delay after >> + * issuing the command. >> + */ >> + if (idata->ic.postsleep_us) >> + udelay(idata->ic.postsleep_us); >> + > > How long can this possibly be? Does usleep make any sense here? > Just to expand further on the question... is this a delay after a command (and it's data transfer, if any) has completely completed and the card is idle as far as the host driver can tell??? Or is the timeout while the card is busy (and holding down DAT)? I popped over to the ASSD simplified spec, and I can see interesting items like Protected_Memory_WR_TIME and WR_SEC_BUSY_TIME, and from the vague description I think that's the timeout going into data.timeout_ns for such transfers. Thanks, A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrei, On Fri, 8 Apr 2011, Andrei Warkentin wrote: > Hi John, > > On Thu, Apr 7, 2011 at 8:18 PM, John Calixto > <john.calixto@modsystems.com> wrote: > > + /* data.flags must already be set before doing this. */ > > + mmc_set_data_timeout(&data, card); > > + /* Allow overriding the timeout_ns for empirical tuning. */ > > + if (idata->ic.force_timeout_ns) > > + data.timeout_ns = idata->ic.force_timeout_ns; > > + > > Are there any R1B ACMDs? > Yes, there are. Do you think this timeout is unnecessary? We found it useful to adjust this to be more accepting of some cheaper cards that exceeded the timeout in the spec (250 ms). > > + mmc_wait_for_req(card->host, &mrq); > > + > > + if (cmd.error) { > > + dev_err(mmc_dev(card->host), "%s: cmd error %d\n", > > + __func__, cmd.error); > > + err = cmd.error; > > + goto acmd_rel_host; > > + } > > + if (data.error) { > > + dev_err(mmc_dev(card->host), "%s: data error %d\n", > > + __func__, data.error); > > + err = data.error; > > + goto acmd_rel_host; > > + } > > + > > + /* > > + * According to the SD specs, some commands require a delay after > > + * issuing the command. > > + */ > > + if (idata->ic.postsleep_us) > > + udelay(idata->ic.postsleep_us); > > + > > How long can this possibly be? Does usleep make any sense here? > In practice, 100 us. Borderline? Note that this sleep is separate from the timeout above. This one is specifically for read operations where there is no such thing as "busy". > > > +struct mmc_ioc_cmd { > > + int write_flag; /* implies direction of data. true = write, false = read */ > > + __u32 opcode; > > + __u32 arg; > > + __u32 response[4]; /* CMD response */ > > + unsigned int flags; > > + unsigned int blksz; > > + unsigned int blocks; > > I'm not sure I understand why blksz is passed. If you needed to set > the block size (via CMD16) prior to some command (like MMC password > protection) I could kind of get it, but then all I see the blk size > getting used for is calculating the data amount in bytes, so you're > not doing that. So why not just infer it from the card queue? > The block size for some commands is strictly specified, and does change according to the command. In these cases, you do not call CMD16 to change it. You just call the command knowing that its block size is fixed according to spec. John
Hi John, On Fri, Apr 8, 2011 at 2:20 PM, John Calixto <john.calixto@modsystems.com> wrote: > Hi Andrei, > > On Fri, 8 Apr 2011, Andrei Warkentin wrote: > >> Hi John, >> >> On Thu, Apr 7, 2011 at 8:18 PM, John Calixto >> <john.calixto@modsystems.com> wrote: >> > + /* data.flags must already be set before doing this. */ >> > + mmc_set_data_timeout(&data, card); >> > + /* Allow overriding the timeout_ns for empirical tuning. */ >> > + if (idata->ic.force_timeout_ns) >> > + data.timeout_ns = idata->ic.force_timeout_ns; >> > + >> >> Are there any R1B ACMDs? >> > > Yes, there are. Do you think this timeout is unnecessary? We found it > useful to adjust this to be more accepting of some cheaper cards that > exceeded the timeout in the spec (250 ms). Is the timeout fixed or variable (I've seen all timeouts in the simplified ASSD to be specified in units of 250ms in fields of ASSD-PR)? Are you using this force_timeout_ns for DAT-transfer commands also, or just for R1B commands? Do you always invoke commands that only involve transfers? If not, I can see how your code handles R1B command timeouts correctly (always pretends it's a data transfer, even if 0-byte sized - thus host driver sets timeout correctly). In my boot partition patch set, there are two changes that add cmd_timeout (for DAT-busy commands) and enable proper handling of it for SDHCI. Obviously, none of the other hosts are fixed, so I guess what you are doing is okay for now. Could you add a "cmd_timeout" field to your ioctl, however, so in the future this is not a breaking change? Then right now you could add something like - if (blksz * blocks == 0) { /* * We're doing a R1B command and relying on the host driver to compute timeout as if * we were doing a data transfer. When all host drivers are fixed to handle R1B right, we won't have to rely * on this behavior, and can make mrq.data NULL in this case, and pass the timeout * through cmd.cmd_timeout. */ data.timeout_ns = idata->cmd_timeout; } and in the future it can be changed to - if (blksz * blocks == 0) { mrq.data = NULL; cmd.cmd_timeout = idata->cmd_timeout; } > > In practice, 100 us. Borderline? Note that this sleep is separate from > the timeout above. This one is specifically for read operations where > there is no such thing as "busy". > You mean 'write'? I don't know. Nasty, really, in my opinion. If you're doing ASSD operations then you know if the card is busy or not (via ASSD-state) . As in something like this paragraph from the simplified ASSD? "The card shall not assert the BUSY signal for time periods longer then the WR_SEC_BUS_BUSY as defined in the card properties register. The only indication for completion of a secure Token execution is the ASSD_STATE field in the card status register. The host shall validate that the card is in the correct state before attempting any ASSD command." If so, maybe it's worthwhile to just make your user code use SEND_PSI to get the ASSD_STATE registers and verify that previous secure operation completed before sending another one. Anyone else have an opinion on this? If this is not ASSD but something else, maybe you have a status register to query someplace? >> I'm not sure I understand why blksz is passed. If you needed to set >> the block size (via CMD16) prior to some command (like MMC password >> protection) I could kind of get it, but then all I see the blk size >> getting used for is calculating the data amount in bytes, so you're >> not doing that. So why not just infer it from the card queue? >> > > The block size for some commands is strictly specified, and does > change according to the command. In these cases, you do not call CMD16 > to change it. You just call the command knowing that its block size is > fixed according to spec. > Ah okay. A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrei, On Fri, 8 Apr 2011, Andrei Warkentin wrote: > Is the timeout fixed or variable (I've seen all timeouts in the > simplified ASSD to be specified in units of 250ms in fields of > ASSD-PR)? Are you using this force_timeout_ns for DAT-transfer > commands also, or just for R1B commands? Actually, ASSD is a different-but-similarly-named-so-it's-confusing set of functionality from that which I am using. Do you have access to "Part 3, Security Specification"? There is no "simplified" version of this spec. As for the usage of the force_timeout_ns, yes, it is also used for DAT-transfer commands. > > Do you always invoke commands that only involve transfers? No. > If not, I can see how your code handles R1B command timeouts correctly > (always pretends it's a data transfer, even if 0-byte sized - thus > host driver sets timeout correctly). In my boot partition patch set, > there are two changes that add cmd_timeout (for DAT-busy commands) and > enable proper handling of it for SDHCI. Obviously, none of the other I'll have a look. > hosts are fixed, so I guess what you are doing is okay for now. Could > you add a "cmd_timeout" field to your ioctl, however, so in the future > this is not a breaking change? Then right now you could add something > like - > > if (blksz * blocks == 0) { > > /* > * We're doing a R1B command and relying on the host driver to > compute timeout as if > * we were doing a data transfer. When all host drivers are fixed > to handle R1B right, we won't have to rely > * on this behavior, and can make mrq.data NULL in this case, and > pass the timeout > * through cmd.cmd_timeout. > */ > data.timeout_ns = idata->cmd_timeout; > } > > and in the future it can be changed to - > > if (blksz * blocks == 0) { > mrq.data = NULL; > cmd.cmd_timeout = idata->cmd_timeout; > } > > Sure. Like you said, this sounds reasonable while there's still variation in timeout setting between the host drivers. > > > > In practice, 100 us. Borderline? Note that this sleep is separate from > > the timeout above. This one is specifically for read operations where > > there is no such thing as "busy". > > > > You mean 'write'? I don't know. Nasty, really, in my opinion. If Unfortunately, I really do mean "read". I suspect it's meant to give the card enough time to do the crypto <shrug>. > you're doing ASSD operations then you know if the card is busy or not > (via ASSD-state) . As in something like this paragraph from the > simplified ASSD? > > "The card shall not assert the BUSY signal for time periods > longer then the WR_SEC_BUS_BUSY as > defined in the card properties register. The only indication > for completion of a secure Token execution is > the ASSD_STATE field in the card status register. The host > shall validate that the card is in the correct > state before attempting any ASSD command." > > If so, maybe it's worthwhile to just make your user code use SEND_PSI > to get the ASSD_STATE registers and verify that previous secure > operation completed before sending another one. > > Anyone else have an opinion on this? > > If this is not ASSD but something else, maybe you have a status > register to query someplace? > > >> I'm not sure I understand why blksz is passed. If you needed to set > >> the block size (via CMD16) prior to some command (like MMC password > >> protection) I could kind of get it, but then all I see the blk size > >> getting used for is calculating the data amount in bytes, so you're > >> not doing that. So why not just infer it from the card queue? > >> I can't quote directly from this spec (it would be nice if some other SDA member could corroborate here... Part 3, anybody???), but it is clear that there is nothing to do but wait. However, speaking to your initial question about usleep() vs. udelay(), I'll try testing with usleep(). John
On Fri, Apr 8, 2011 at 4:25 PM, John Calixto <john.calixto@modsystems.com> wrote: > Hi Andrei, > > On Fri, 8 Apr 2011, Andrei Warkentin wrote: >> Is the timeout fixed or variable (I've seen all timeouts in the >> simplified ASSD to be specified in units of 250ms in fields of >> ASSD-PR)? Are you using this force_timeout_ns for DAT-transfer >> commands also, or just for R1B commands? > > Actually, ASSD is a different-but-similarly-named-so-it's-confusing set > of functionality from that which I am using. Do you have access to > "Part 3, Security Specification"? There is no "simplified" version of > this spec. > > As for the usage of the force_timeout_ns, yes, it is also used for > DAT-transfer commands. > >> >> Do you always invoke commands that only involve transfers? > > No. > >> If not, I can see how your code handles R1B command timeouts correctly >> (always pretends it's a data transfer, even if 0-byte sized - thus >> host driver sets timeout correctly). In my boot partition patch set, >> there are two changes that add cmd_timeout (for DAT-busy commands) and >> enable proper handling of it for SDHCI. Obviously, none of the other > > I'll have a look. > >> hosts are fixed, so I guess what you are doing is okay for now. Could >> you add a "cmd_timeout" field to your ioctl, however, so in the future >> this is not a breaking change? Then right now you could add something >> like - >> >> if (blksz * blocks == 0) { >> >> /* >> * We're doing a R1B command and relying on the host driver to >> compute timeout as if >> * we were doing a data transfer. When all host drivers are fixed >> to handle R1B right, we won't have to rely >> * on this behavior, and can make mrq.data NULL in this case, and >> pass the timeout >> * through cmd.cmd_timeout. >> */ >> data.timeout_ns = idata->cmd_timeout; >> } >> >> and in the future it can be changed to - >> >> if (blksz * blocks == 0) { >> mrq.data = NULL; >> cmd.cmd_timeout = idata->cmd_timeout; >> } >> >> > > Sure. Like you said, this sounds reasonable while there's still > variation in timeout setting between the host drivers. > >> > >> > In practice, 100 us. Borderline? Note that this sleep is separate from >> > the timeout above. This one is specifically for read operations where >> > there is no such thing as "busy". >> > >> >> You mean 'write'? I don't know. Nasty, really, in my opinion. If > > Unfortunately, I really do mean "read". I suspect it's meant to give > the card enough time to do the crypto <shrug>. > >> you're doing ASSD operations then you know if the card is busy or not >> (via ASSD-state) . As in something like this paragraph from the >> simplified ASSD? >> >> "The card shall not assert the BUSY signal for time periods >> longer then the WR_SEC_BUS_BUSY as >> defined in the card properties register. The only indication >> for completion of a secure Token execution is >> the ASSD_STATE field in the card status register. The host >> shall validate that the card is in the correct >> state before attempting any ASSD command." >> >> If so, maybe it's worthwhile to just make your user code use SEND_PSI >> to get the ASSD_STATE registers and verify that previous secure >> operation completed before sending another one. >> >> Anyone else have an opinion on this? >> >> If this is not ASSD but something else, maybe you have a status >> register to query someplace? >> >> >> I'm not sure I understand why blksz is passed. If you needed to set >> >> the block size (via CMD16) prior to some command (like MMC password >> >> protection) I could kind of get it, but then all I see the blk size >> >> getting used for is calculating the data amount in bytes, so you're >> >> not doing that. So why not just infer it from the card queue? >> >> > > I can't quote directly from this spec (it would be nice if some other > SDA member could corroborate here... Part 3, anybody???), but it is > clear that there is nothing to do but wait. However, speaking to your > initial question about usleep() vs. udelay(), I'll try testing with > usleep(). > > John I'm not sure what the etiquette for this list is, but you can a Reviewed-by: Andrei Warkentin <andreiw@motorola.com> ...if that's alright with the maintainers. A -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Fri, Apr 08 2011, Andrei Warkentin wrote: > I'm not sure what the etiquette for this list is, but you can a > Reviewed-by: Andrei Warkentin <andreiw@motorola.com> > > ...if that's alright with the maintainers. More than fine; thanks very much for the review, Andrei. - Chris.
On Fri, 8 Apr 2011, Andrei Warkentin wrote: > > I'm not sure what the etiquette for this list is, but you can a > Reviewed-by: Andrei Warkentin <andreiw@motorola.com> > > ...if that's alright with the maintainers. > Will do - Thanks for taking the time Andrei! John -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi John, On Thu, Apr 07 2011, John Calixto wrote: > Sending ACMDs from userspace is useful for such things as: > > - The security application of an SD card (SD Specification, Part 3, > Security) > > - SD passthrough for virtual machines > > Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 > SoC, TI OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC. > > Signed-off-by: John Calixto <john.calixto@modsystems.com> This adds two gcc warnings here (compiling for ARM): CC drivers/mmc/card/block.o drivers/mmc/card/block.c: In function ‘mmc_blk_ioctl_copy_from_user’: drivers/mmc/card/block.c:199:33: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] drivers/mmc/card/block.c: In function ‘mmc_blk_ioctl_acmd’: drivers/mmc/card/block.c:308:20: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] ... > + if (copy_from_user(idata->buf, (u8 *) user->data_ptr, idata->buf_bytes)) { > + err = -EFAULT; > + goto copy_err; > + } ... > + if (copy_to_user((u8 *) ic_ptr->data_ptr, idata->buf, idata->buf_bytes)) { > + err = -EFAULT; > + goto acmd_rel_host; > + } ... Thanks, - Chris.
On Mon, 11 Apr 2011, Chris Ball wrote: > This adds two gcc warnings here (compiling for ARM): > > CC drivers/mmc/card/block.o > drivers/mmc/card/block.c: In function ‘mmc_blk_ioctl_copy_from_user’: > drivers/mmc/card/block.c:199:33: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > drivers/mmc/card/block.c: In function ‘mmc_blk_ioctl_acmd’: > drivers/mmc/card/block.c:308:20: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > > ... > > + if (copy_from_user(idata->buf, (u8 *) user->data_ptr, idata->buf_bytes)) { > > + err = -EFAULT; > > + goto copy_err; > > + } > ... > > + if (copy_to_user((u8 *) ic_ptr->data_ptr, idata->buf, idata->buf_bytes)) { > > + err = -EFAULT; > > + goto acmd_rel_host; > > + } > ... > Hrmmm... So, what's the correct way to do this compat_ioctl() stuff? My first attempt at a compat_ioctl() implementation in v2 used a 32bit version of the structure to marshall the data. That was shot down as being unnecessarily complicated since the buffer pointer was the only thing that was not compat friendly. It was recommended that I could just make this a 64bit int field and let the callers do the appropriate casting. However, this latter solution (implemented in v3) causes the compiler warning that you describe above. Should I revert to the v2 implementation? John
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index 61d233a..4ee16ca 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -31,7 +31,10 @@ #include <linux/mutex.h> #include <linux/scatterlist.h> #include <linux/string_helpers.h> +#include <linux/delay.h> +#include <linux/capability.h> +#include <linux/mmc/ioctl.h> #include <linux/mmc/card.h> #include <linux/mmc/host.h> #include <linux/mmc/mmc.h> @@ -158,11 +161,192 @@ mmc_blk_getgeo(struct block_device *bdev, struct hd_geometry *geo) return 0; } +struct mmc_blk_ioc_data { + struct mmc_ioc_cmd ic; + unsigned char *buf; + size_t buf_bytes; +}; + +static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user( + struct mmc_ioc_cmd __user *user) +{ + struct mmc_blk_ioc_data *idata; + int err; + + idata = kzalloc(sizeof(*idata), GFP_KERNEL); + if (!idata) { + err = -ENOMEM; + goto copy_err; + } + + if (copy_from_user(&idata->ic, user, sizeof(idata->ic))) { + err = -EFAULT; + goto copy_err; + } + + idata->buf_bytes = idata->ic.blksz * idata->ic.blocks; + + idata->buf = kzalloc(idata->buf_bytes, GFP_KERNEL); + if (!idata->buf) { + err = -ENOMEM; + goto copy_err; + } + + if (copy_from_user(idata->buf, (u8 *) user->data_ptr, idata->buf_bytes)) { + err = -EFAULT; + goto copy_err; + } + + return idata; + +copy_err: + kfree(idata->buf); + kfree(idata); + return ERR_PTR(err); + +} + +static int mmc_blk_ioctl_acmd(struct block_device *bdev, + struct mmc_ioc_cmd __user *ic_ptr) +{ + struct mmc_blk_ioc_data *idata; + struct mmc_blk_data *md; + struct mmc_card *card; + struct mmc_command cmd = {0}; + struct mmc_data data = {0}; + struct mmc_request mrq = {0}; + struct scatterlist sg; + int err; + + /* + * The caller must have CAP_SYS_RAWIO, and must be calling this on the + * whole block device, not on a partition. This prevents overspray + * between sibling partitions. + */ + if ((!capable(CAP_SYS_RAWIO)) || (bdev != bdev->bd_contains)) + return -EPERM; + + idata = mmc_blk_ioctl_copy_from_user(ic_ptr); + if (IS_ERR(idata)) + return PTR_ERR(idata); + + cmd.opcode = idata->ic.opcode; + cmd.arg = idata->ic.arg; + cmd.flags = idata->ic.flags; + + data.sg = &sg; + data.sg_len = 1; + data.blksz = idata->ic.blksz; + data.blocks = idata->ic.blocks; + + sg_init_one(data.sg, idata->buf, idata->buf_bytes); + + if (idata->ic.write_flag) + data.flags = MMC_DATA_WRITE; + else + data.flags = MMC_DATA_READ; + + mrq.cmd = &cmd; + mrq.data = &data; + + md = mmc_blk_get(bdev->bd_disk); + if (!md) { + err = -EINVAL; + goto acmd_done; + } + + card = md->queue.card; + if (IS_ERR(card)) { + err = PTR_ERR(card); + goto acmd_done; + } + + mmc_claim_host(card->host); + + err = mmc_app_cmd(card->host, card); + if (err) + goto acmd_rel_host; + + /* data.flags must already be set before doing this. */ + mmc_set_data_timeout(&data, card); + /* Allow overriding the timeout_ns for empirical tuning. */ + if (idata->ic.force_timeout_ns) + data.timeout_ns = idata->ic.force_timeout_ns; + + mmc_wait_for_req(card->host, &mrq); + + if (cmd.error) { + dev_err(mmc_dev(card->host), "%s: cmd error %d\n", + __func__, cmd.error); + err = cmd.error; + goto acmd_rel_host; + } + if (data.error) { + dev_err(mmc_dev(card->host), "%s: data error %d\n", + __func__, data.error); + err = data.error; + goto acmd_rel_host; + } + + /* + * According to the SD specs, some commands require a delay after + * issuing the command. + */ + if (idata->ic.postsleep_us) + udelay(idata->ic.postsleep_us); + + if (copy_to_user(&(ic_ptr->response), cmd.resp, sizeof(cmd.resp))) { + err = -EFAULT; + goto acmd_rel_host; + } + + if (!idata->ic.write_flag) { + if (copy_to_user((u8 *) ic_ptr->data_ptr, idata->buf, idata->buf_bytes)) { + err = -EFAULT; + goto acmd_rel_host; + } + } + +acmd_rel_host: + mmc_release_host(card->host); + +acmd_done: + mmc_blk_put(md); + kfree(idata->buf); + kfree(idata); + return err; +} + +static int mmc_blk_ioctl(struct block_device *bdev, fmode_t mode, + unsigned int cmd, unsigned long arg) +{ + int ret = -EINVAL; + mutex_lock(&block_mutex); + if (cmd == MMC_IOC_ACMD) + ret = mmc_blk_ioctl_acmd(bdev, (struct mmc_ioc_cmd __user *)arg); + mutex_unlock(&block_mutex); + return ret; +} + +#ifdef CONFIG_COMPAT +static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode, + unsigned int cmd, unsigned long arg) +{ + struct mmc_ioc_cmd __user *ic = (struct mmc_ioc_cmd __user *) arg; + ic->data_ptr = ic->data_ptr & 0xffffffff; + return mmc_blk_ioctl(bdev, mode, cmd, arg); +} +#endif + static const struct block_device_operations mmc_bdops = { .open = mmc_blk_open, .release = mmc_blk_release, .getgeo = mmc_blk_getgeo, .owner = THIS_MODULE, + .ioctl = mmc_blk_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = mmc_blk_compat_ioctl, +#endif }; struct mmc_blk_request { diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c index 797cdb5..990dd43 100644 --- a/drivers/mmc/core/sd_ops.c +++ b/drivers/mmc/core/sd_ops.c @@ -20,7 +20,7 @@ #include "core.h" #include "sd_ops.h" -static int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) +int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) { int err; struct mmc_command cmd; @@ -48,6 +48,7 @@ static int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card) return 0; } +EXPORT_SYMBOL_GPL(mmc_app_cmd); /** * mmc_wait_for_app_cmd - start an application command and wait for diff --git a/include/linux/Kbuild b/include/linux/Kbuild index 75cf611..ed38527 100644 --- a/include/linux/Kbuild +++ b/include/linux/Kbuild @@ -4,6 +4,7 @@ header-y += caif/ header-y += dvb/ header-y += hdlc/ header-y += isdn/ +header-y += mmc/ header-y += nfsd/ header-y += raid/ header-y += spi/ diff --git a/include/linux/mmc/Kbuild b/include/linux/mmc/Kbuild new file mode 100644 index 0000000..1fb2644 --- /dev/null +++ b/include/linux/mmc/Kbuild @@ -0,0 +1 @@ +header-y += ioctl.h diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h index 07f27af..bfc6127 100644 --- a/include/linux/mmc/core.h +++ b/include/linux/mmc/core.h @@ -133,6 +133,7 @@ struct mmc_card; extern void mmc_wait_for_req(struct mmc_host *, struct mmc_request *); extern int mmc_wait_for_cmd(struct mmc_host *, struct mmc_command *, int); +extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *); extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *, struct mmc_command *, int); diff --git a/include/linux/mmc/ioctl.h b/include/linux/mmc/ioctl.h new file mode 100644 index 0000000..0feb2f2 --- /dev/null +++ b/include/linux/mmc/ioctl.h @@ -0,0 +1,16 @@ +#ifndef _MMC_IOCTL_H +#define _MMC_IOCTL_H +struct mmc_ioc_cmd { + int write_flag; /* implies direction of data. true = write, false = read */ + __u32 opcode; + __u32 arg; + __u32 response[4]; /* CMD response */ + unsigned int flags; + unsigned int blksz; + unsigned int blocks; + unsigned int postsleep_us; /* apply usecond delay *after* issuing command */ + unsigned int force_timeout_ns; /* force timeout to be force_timeout_ns ns */ + __u64 data_ptr; /* DAT buffer */ +}; +#define MMC_IOC_ACMD _IOWR(MMC_BLOCK_MAJOR, 0, struct mmc_ioc_cmd) +#endif /* _MMC_IOCTL_H */
Sending ACMDs from userspace is useful for such things as: - The security application of an SD card (SD Specification, Part 3, Security) - SD passthrough for virtual machines Tested on TI PCIxx12 (SDHCI), Sigma Designs SMP8652 SoC, TI OMAP3621 SoC, TI OMAP3630 SoC, Samsung S5PC110 SoC, Qualcomm MSM7200A SoC. Signed-off-by: John Calixto <john.calixto@modsystems.com> --- drivers/mmc/card/block.c | 184 +++++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/core/sd_ops.c | 3 +- include/linux/Kbuild | 1 + include/linux/mmc/Kbuild | 1 + include/linux/mmc/core.h | 1 + include/linux/mmc/ioctl.h | 16 ++++ 6 files changed, 205 insertions(+), 1 deletions(-) create mode 100644 include/linux/mmc/Kbuild create mode 100644 include/linux/mmc/ioctl.h