/* * HD-audio core bus driver */ #include #include #include #include #include #include "trace.h" static void process_unsol_events(struct work_struct *work); static const struct hdac_bus_ops default_ops = { .command = snd_hdac_bus_send_cmd, .get_response = snd_hdac_bus_get_response, }; /** * snd_hdac_bus_init - initialize a HD-audio bas bus * @bus: the pointer to bus object * @ops: bus verb operators * @io_ops: lowlevel I/O operators * * Returns 0 if successful, or a negative error code. */ int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev, const struct hdac_bus_ops *ops, const struct hdac_io_ops *io_ops) { memset(bus, 0, sizeof(*bus)); bus->dev = dev; if (ops) bus->ops = ops; else bus->ops = &default_ops; bus->io_ops = io_ops; INIT_LIST_HEAD(&bus->stream_list); INIT_LIST_HEAD(&bus->codec_list); INIT_WORK(&bus->unsol_work, process_unsol_events); spin_lock_init(&bus->reg_lock); mutex_init(&bus->cmd_mutex); bus->irq = -1; return 0; } EXPORT_SYMBOL_GPL(snd_hdac_bus_init); /** * snd_hdac_bus_exit - clean up a HD-audio bas bus * @bus: the pointer to bus object */ void snd_hdac_bus_exit(struct hdac_bus *bus) { WARN_ON(!list_empty(&bus->stream_list)); WARN_ON(!list_empty(&bus->codec_list)); cancel_work_sync(&bus->unsol_work); } EXPORT_SYMBOL_GPL(snd_hdac_bus_exit); /** * snd_hdac_bus_exec_verb - execute a HD-audio verb on the given bus * @bus: bus object * @cmd: HD-audio encoded verb * @res: pointer to store the response, NULL if performing asynchronously * * Returns 0 if successful, or a negative error code. */ int snd_hdac_bus_exec_verb(struct hdac_bus *bus, unsigned int addr, unsigned int cmd, unsigned int *res) { int err; mutex_lock(&bus->cmd_mutex); err = snd_hdac_bus_exec_verb_unlocked(bus, addr, cmd, res); mutex_unlock(&bus->cmd_mutex); return err; } EXPORT_SYMBOL_GPL(snd_hdac_bus_exec_verb); /** * snd_hdac_bus_exec_verb_unlocked - unlocked version * @bus: bus object * @cmd: HD-audio encoded verb * @res: pointer to store the response, NULL if performing asynchronously * * Returns 0 if successful, or a negative error code. */ int snd_hdac_bus_exec_verb_unlocked(struct hdac_bus *bus, unsigned int addr, unsigned int cmd, unsigned int *res) { unsigned int tmp; int err; if (cmd == ~0) return -EINVAL; if (res) *res = -1; else if (bus->sync_write) res = &tmp; for (;;) { trace_hda_send_cmd(bus, cmd); err = bus->ops->command(bus, cmd); if (err != -EAGAIN) break; /* process pending verbs */ err = bus->ops->get_response(bus, addr, &tmp); if (err) break; } if (!err && res) { err = bus->ops->get_response(bus, addr, res); trace_hda_get_response(bus, addr, *res); } return err; } EXPORT_SYMBOL_GPL(snd_hdac_bus_exec_verb_unlocked); /** * snd_hdac_bus_queue_event - add an unsolicited event to queue * @bus: the BUS * @res: unsolicited event (lower 32bit of RIRB entry) * @res_ex: codec addr and flags (upper 32bit or RIRB entry) * * Adds the given event to the queue. The events are processed in * the workqueue asynchronously. Call this function in the interrupt * hanlder when RIRB receives an unsolicited event. */ void snd_hdac_bus_queue_event(struct hdac_bus *bus, u32 res, u32 res_ex) { unsigned int wp; if (!bus) return; trace_hda_unsol_event(bus, res, res_ex); wp = (bus->unsol_wp + 1) % HDA_UNSOL_QUEUE_SIZE; bus->unsol_wp = wp; wp <<= 1; bus->unsol_queue[wp] = res; bus->unsol_queue[wp + 1] = res_ex; schedule_work(&bus->unsol_work); } EXPORT_SYMBOL_GPL(snd_hdac_bus_queue_event); /* * process queued unsolicited events */ static void process_unsol_events(struct work_struct *work) { struct hdac_bus *bus = container_of(work, struct hdac_bus, unsol_work); struct hdac_device *codec; struct hdac_driver *drv; unsigned int rp, caddr, res; while (bus->unsol_rp != bus->unsol_wp) { rp = (bus->unsol_rp + 1) % HDA_UNSOL_QUEUE_SIZE; bus->unsol_rp = rp; rp <<= 1; res = bus->unsol_queue[rp]; caddr = bus->unsol_queue[rp + 1]; if (!(caddr & (1 << 4))) /* no unsolicited event? */ continue; codec = bus->caddr_tbl[caddr & 0x0f]; if (!codec || !codec->dev.driver) continue; drv = drv_to_hdac_driver(codec->dev.driver); if (drv->unsol_event) drv->unsol_event(codec, res); } } /** * snd_hdac_bus_add_device - Add a codec to bus * @bus: HDA core bus * @codec: HDA core device to add * * Adds the given codec to the list in the bus. The caddr_tbl array * and codec_powered bits are updated, as well. * Returns zero if success, or a negative error code. */ int snd_hdac_bus_add_device(struct hdac_bus *bus, struct hdac_device *codec) { if (bus->caddr_tbl[codec->addr]) { dev_err(bus->dev, "address 0x%x is already occupied\n", codec->addr); return -EBUSY; } list_add_tail(&codec->list, &bus->codec_list); bus->caddr_tbl[codec->addr] = codec; set_bit(codec->addr, &bus->codec_powered); bus->num_codecs++; return 0; } EXPORT_SYMBOL_GPL(snd_hdac_bus_add_device); /** * snd_hdac_bus_remove_device - Remove a codec from bus * @bus: HDA core bus * @codec: HDA core device to remove */ void snd_hdac_bus_remove_device(struct hdac_bus *bus, struct hdac_device *codec) { WARN_ON(bus != codec->bus); if (list_empty(&codec->list)) return; list_del_init(&codec->list); bus->caddr_tbl[codec->addr] = NULL; clear_bit(codec->addr, &bus->codec_powered); bus->num_codecs--; } EXPORT_SYMBOL_GPL(snd_hdac_bus_remove_device); oseconds conversion is using signed math, but the delta is unsigned. This makes the conversion space smaller than necessary and in case of a multiplication overflow the conversion can become negative. The conversion is done with scaled math: s64 nsec_delta = ((s64)clkdelta * clk->mult) >> clk->shift; Shifting a signed integer right obvioulsy preserves the sign, which has interesting consequences: - Time jumps backwards - __iter_div_u64_rem() which is used in one of the calling code pathes will take forever to piecewise calculate the seconds/nanoseconds part. This has been reported by several people with different scenarios: David observed that when stopping a VM with a debugger: "It was essentially the stopped by debugger case. I forget exactly why, but the guest was being explicitly stopped from outside, it wasn't just scheduling lag. I think it was something in the vicinity of 10 minutes stopped." When lifting the stop the machine went dead. The stopped by debugger case is not really interesting, but nevertheless it would be a good thing not to die completely. But this was also observed on a live system by Liav: "When the OS is too overloaded, delta will get a high enough value for the msb of the sum delta * tkr->mult + tkr->xtime_nsec to be set, and so after the shift the nsec variable will gain a value similar to 0xffffffffff000000." Unfortunately this has been reintroduced recently with commit 6bd58f09e1d8 ("time: Add cycles to nanoseconds translation"). It had been fixed a year ago already in commit 35a4933a8959 ("time: Avoid signed overflow in timekeeping_get_ns()"). Though it's not surprising that the issue has been reintroduced because the function itself and the whole call chain uses s64 for the result and the propagation of it. The change in this recent commit is subtle: s64 nsec; - nsec = (d * m + n) >> s: + nsec = d * m + n; + nsec >>= s; d being type of cycle_t adds another level of obfuscation. This wouldn't have happened if the previous change to unsigned computation would have made the 'nsec' variable u64 right away and a follow up patch had cleaned up the whole call chain. There have been patches submitted which basically did a revert of the above patch leaving everything else unchanged as signed. Back to square one. This spawned a admittedly pointless discussion about potential users which rely on the unsigned behaviour until someone pointed out that it had been fixed before. The changelogs of said patches added further confusion as they made finally false claims about the consequences for eventual users which expect signed results. Despite delta being cycle_t, aka. u64, it's very well possible to hand in a signed negative value and the signed computation will happily return the correct result. But nobody actually sat down and analyzed the code which was added as user after the propably unintended signed conversion. Though in sensitive code like this it's better to analyze it proper and make sure that nothing relies on this than hunting the subtle wreckage half a year later. After analyzing all call chains it stands that no caller can hand in a negative value (which actually would work due to the s64 cast) and rely on the signed math to do the right thing. Change the conversion function to unsigned math. The conversion of all call chains is done in a follow up patch. This solves the starvation issue, which was caused by the negative result, but it does not solve the underlying problem. It merily procrastinates it. When the timekeeper update is deferred long enough that the unsigned multiplication overflows, then time going backwards is observable again. It does neither solve the issue of clocksources with a small counter width which will wrap around possibly several times and cause random time stamps to be generated. But those are usually not found on systems used for virtualization, so this is likely a non issue. I took the liberty to claim authorship for this simply because analyzing all callsites and writing the changelog took substantially more time than just making the simple s/s64/u64/ change and ignore the rest. Fixes: 6bd58f09e1d8 ("time: Add cycles to nanoseconds translation") Reported-by: David Gibson <david@gibson.dropbear.id.au> Reported-by: Liav Rehana <liavr@mellanox.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: Parit Bhargava <prarit@redhat.com> Cc: Laurent Vivier <lvivier@redhat.com> Cc: "Christopher S. Hall" <christopher.s.hall@intel.com> Cc: Chris Metcalf <cmetcalf@mellanox.com> Cc: Richard Cochran <richardcochran@gmail.com> Cc: John Stultz <john.stultz@linaro.org> Cc: stable@vger.kernel.org Link: http://lkml.kernel.org/r/20161208204228.688545601@linutronix.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Diffstat (limited to 'kernel')