Compare commits

...

6 Commits

Author SHA1 Message Date
Yu Watanabe 0119ab3d6c
Merge pull request #16476 from keszybz/qemu-autosuspend-rules
Add autosuspend rules for emulated QEMU devices
2020-07-18 09:10:20 +09:00
Zbigniew Jędrzejewski-Szmek 77547d5313 hwdb: check that uppercase digits are used in modalias patterns
This is all confusing as hell, becuase in some places lowercase hexadecimal
digits are used, and in other places uppercase. This adds a check for the
most common case that we and others got wrong.

I tried to extend the general grammar in hwdb_grammar() to include this check,
but it quickly became very complicated and didn't seem to work properly. Doing
initial parsing with more general rules is easier and also seems to give better
error messages:

/home/zbyszek/src/systemd-work/build/../hwdb.d/60-autosuspend.hwdb: 3 match groups, 5 matches, 3 properties
Pattern 'v058fp9540*' is invalid: Expected W:(0123...), found 'f'  (at char 4), (line:1, col:5)
2020-07-17 11:15:58 +02:00
Zbigniew Jędrzejewski-Szmek 457763aa03 hwdb: allow spaces in usb: matches and similar patterns
In the past we didn't have any matches like that, so the parser was stricter
than necessary, but now we have, so allow that.
2020-07-17 07:44:10 +02:00
Zbigniew Jędrzejewski-Szmek ec8bebbcc2 Add autosuspend rules for emulated QEMU devices
This effectively partially reverts "rules: remove all power management from
udev" / e2452eef02. The rules for emulated QEMU
hardware were removed in one fell swoop with other rules which were causing
problems. But the qemu rules were working properly (and were adjusted through
patches over time). Nowadays we have a hwdb for this, so add hwdb entries using
the new detailed modalias.

https://github.com/systemd/systemd/pull/353#issuecomment-658810289
2020-07-16 19:00:26 +02:00
Zbigniew Jędrzejewski-Szmek df7667323d udev: change the modalias string for usb devices to include the device name
When the kernel does not provide a modalias, we generate our own for usb devices.
For some reason, we generated the expected usb:vXXXXpYYYY string, suffixed by "*".
It was added that way already in 796b06c21b, but I
think that was a mistake, and Kay was thinking about the match pattern instead
of the matched string.

For example, for a qemu device:
old: "usb:v0627p0001*"
new: "usb:v0627p0001:QEMU USB Tablet"

On the match side, all hwdb files in the wild seem to be using match patterns
with "*" at the end. So we can add more stuff to our generated modalias with
impunity.

This will allow more obvious and more certain matches on USB devices. In
principle the vendor+product id should be unique, but it's only 8 digits, and
there's a high chance of people getting this wrong. And matching the wrong
device would be quite problematic. By including the name in the match string we
make a mismatch much less likely.
2020-07-16 19:00:26 +02:00
Zbigniew Jędrzejewski-Szmek 19b4864346 hwdb/autosuspend: add missing parenthesis 2020-07-16 18:06:35 +02:00
4 changed files with 44 additions and 9 deletions

View File

@ -36,6 +36,16 @@
usb:v058Fp9540* usb:v058Fp9540*
ID_AUTOSUSPEND=1 ID_AUTOSUSPEND=1
#########################################
# QEMU
#########################################
# Emulated USB HID devices
usb:v0627p0001:*QEMU USB Keyboard*
usb:v0627p0001:*QEMU USB Mouse*
usb:v0627p0001:*QEMU USB Tablet*
ID_AUTOSUSPEND=1
######################################### #########################################
# Wacom # Wacom
######################################### #########################################

View File

@ -79,6 +79,9 @@ GENERAL_MATCHES = {'acpi',
'OUI', 'OUI',
} }
def upperhex_word(length):
return Word(nums + 'ABCDEF', exact=length)
@lru_cache() @lru_cache()
def hwdb_grammar(): def hwdb_grammar():
ParserElement.setDefaultWhitespaceChars('') ParserElement.setDefaultWhitespaceChars('')
@ -87,7 +90,7 @@ def hwdb_grammar():
for category, conn in TYPES.items()) for category, conn in TYPES.items())
matchline_typed = Combine(prefix + Word(printables + ' ' + '®')) matchline_typed = Combine(prefix + Word(printables + ' ' + '®'))
matchline_general = Combine(Or(GENERAL_MATCHES) + ':' + Word(printables)) matchline_general = Combine(Or(GENERAL_MATCHES) + ':' + Word(printables + ' ' + '®'))
matchline = (matchline_typed | matchline_general) + EOL matchline = (matchline_typed | matchline_general) + EOL
propertyline = (White(' ', exact=1).suppress() + propertyline = (White(' ', exact=1).suppress() +
@ -180,8 +183,27 @@ def parse(fname):
return [] return []
return [convert_properties(g) for g in parsed.GROUPS] return [convert_properties(g) for g in parsed.GROUPS]
def check_match_uniqueness(groups): def check_matches(groups):
matches = sum((group[0] for group in groups), []) matches = sum((group[0] for group in groups), [])
# This is a partial check. The other cases could be also done, but those
# two are most commonly wrong.
grammars = { 'usb' : 'v' + upperhex_word(4) + Optional('p' + upperhex_word(4)),
'pci' : 'v' + upperhex_word(8) + Optional('d' + upperhex_word(8)),
}
for match in matches:
prefix, rest = match.split(':', maxsplit=1)
gr = grammars.get(prefix)
if gr:
try:
gr.parseString(rest)
except ParseBaseException as e:
error('Pattern {!r} is invalid: {}', rest, e)
continue
if rest[-1] not in '*:':
error('pattern {} does not end with "*" or ":"', match)
matches.sort() matches.sort()
prev = None prev = None
for match in matches: for match in matches:
@ -256,7 +278,7 @@ if __name__ == '__main__':
for fname in args: for fname in args:
groups = parse(fname) groups = parse(fname)
print_summary(fname, groups) print_summary(fname, groups)
check_match_uniqueness(groups) check_matches(groups)
check_properties(groups) check_properties(groups)
sys.exit(ERROR) sys.exit(ERROR)

View File

@ -47,7 +47,7 @@ int udev_builtin_hwdb_lookup(sd_device *dev,
} }
static const char *modalias_usb(sd_device *dev, char *s, size_t size) { static const char *modalias_usb(sd_device *dev, char *s, size_t size) {
const char *v, *p; const char *v, *p, *n = NULL;
uint16_t vn, pn; uint16_t vn, pn;
if (sd_device_get_sysattr_value(dev, "idVendor", &v) < 0) if (sd_device_get_sysattr_value(dev, "idVendor", &v) < 0)
@ -58,15 +58,16 @@ static const char *modalias_usb(sd_device *dev, char *s, size_t size) {
return NULL; return NULL;
if (safe_atoux16(p, &pn) < 0) if (safe_atoux16(p, &pn) < 0)
return NULL; return NULL;
snprintf(s, size, "usb:v%04Xp%04X*", vn, pn); (void) sd_device_get_sysattr_value(dev, "product", &n);
snprintf(s, size, "usb:v%04Xp%04X:%s", vn, pn, strempty(n));
return s; return s;
} }
static int udev_builtin_hwdb_search(sd_device *dev, sd_device *srcdev, static int udev_builtin_hwdb_search(sd_device *dev, sd_device *srcdev,
const char *subsystem, const char *prefix, const char *subsystem, const char *prefix,
const char *filter, bool test) { const char *filter, bool test) {
sd_device *d; char s[LINE_MAX];
char s[16];
bool last = false; bool last = false;
int r = 0; int r = 0;
@ -75,7 +76,7 @@ static int udev_builtin_hwdb_search(sd_device *dev, sd_device *srcdev,
if (!srcdev) if (!srcdev)
srcdev = dev; srcdev = dev;
for (d = srcdev; d; ) { for (sd_device *d = srcdev; d; ) {
const char *dsubsys, *devtype, *modalias = NULL; const char *dsubsys, *devtype, *modalias = NULL;
if (sd_device_get_subsystem(d, &dsubsys) < 0) if (sd_device_get_subsystem(d, &dsubsys) < 0)
@ -101,6 +102,8 @@ static int udev_builtin_hwdb_search(sd_device *dev, sd_device *srcdev,
if (!modalias) if (!modalias)
goto next; goto next;
log_device_debug(dev, "hwdb modalias key: \"%s\"", modalias);
r = udev_builtin_hwdb_lookup(dev, prefix, modalias, filter, test); r = udev_builtin_hwdb_lookup(dev, prefix, modalias, filter, test);
if (r > 0) if (r > 0)
break; break;

View File

@ -14,7 +14,7 @@ for entry in chromiumos.gen_autosuspend_rules.PCI_IDS:
device = int(device, 16) device = int(device, 16)
print('pci:v{:08X}d{:08X}*'.format(vendor, device)) print('pci:v{:08X}d{:08X}*'.format(vendor, device))
print('# usb:v<VEND>p<PROD> (4 uppercase hexadecimal digits twice') print('# usb:v<VEND>p<PROD> (4 uppercase hexadecimal digits twice)')
for entry in chromiumos.gen_autosuspend_rules.USB_IDS: for entry in chromiumos.gen_autosuspend_rules.USB_IDS:
vendor, product = entry.split(':') vendor, product = entry.split(':')
vendor = int(vendor, 16) vendor = int(vendor, 16)