| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-14 | |||
| 14:24:14 | sean-k-mooney | although when i suggested adding namespaces in os-traits intially i was hoping to not use __init__.py | |
| 14:24:19 | edleafe | If you're creating a directory hierarchy, and need directory-level traits, the only way to do that is in __init__.py | |
| 14:24:46 | sean-k-mooney | edleafe: yes which is why we use it | |
| 14:25:12 | openstackgerrit | Matthew Booth proposed openstack/nova master: Fix retry of instance_update_and_get_original https://review.opendev.org/658845 | |
| 14:25:26 | efried | edleafe: you can't have x86.py and x86/ ? | |
| 14:25:28 | sean-k-mooney | but do we need hw/cpu/x86/intel.py or just hw/cpu/intel.py | |
| 14:25:40 | sean-k-mooney | if its an intel specific trait why add x86 | |
| 14:25:43 | efried | edleafe: that seems horrible to me because you wouldn't know where to look for your stuff, but is it even legal python? | |
| 14:25:45 | mdbooth | bauzas stephenfin: ^^^ Only change is whitespace for stephenfin's nit. | |
| 14:25:47 | kashyap | efried: edleafe: So, does this structure make sense: | |
| 14:25:49 | kashyap | --- | |
| 14:25:49 | kashyap | hw/cpu/x86/__init__.py (Common for both AMD and Intel) | |
| 14:25:49 | kashyap | hw/cpu/x86/intel.py (Intel-specific) | |
| 14:25:49 | kashyap | hw/cpu/x86/amd.py (AMD-specific) | |
| 14:25:51 | kashyap | hw/cpu/amd.py (Deprecate it with a comment) | |
| 14:26:04 | efried | kashyap: Yes, I think that's the right way to go. | |
| 14:26:14 | edleafe | efried: that would create HW_CPU_X86_X86_FOO, no? | |
| 14:26:14 | sean-k-mooney | i think the opisite | |
| 14:26:26 | efried | kashyap: And remove x86.py. Current contents go to x86/__init__.py | |
| 14:26:26 | kashyap | sean-k-mooney: Also note that there are other vulns. _besides_ Spectre/Meltdown: E.g. "L1TF". | |
| 14:26:27 | sean-k-mooney | i would prefer to add an intel.py beside the amd.py | |
| 14:26:37 | edleafe | efried: oh, you mean at the level above? | |
| 14:26:42 | stephenfin | This feels like a lot of ado about nothing. What's wrong with keeping everything in x86.py again? | |
| 14:26:46 | kashyap | efried: Right (on removing x86.py) | |
| 14:26:49 | edleafe | efried: yeah, that could work | |
| 14:26:54 | sean-k-mooney | i mean just add an intel.py here https://github.com/openstack/os-traits/tree/master/os_traits/hw/cpu | |
| 14:27:00 | edleafe | But it's unnecesary | |
| 14:27:07 | efried | edleafe: I meant, is it legal to have: hw/cpu/x86.py and hw/cpu/x86/__init__.py | |
| 14:27:08 | aspiers | stephenfin: stop trying to spoil the bike-shedding fun ;-) | |
| 14:27:20 | stephenfin | :) | |
| 14:27:21 | kashyap | stephenfin: "Nothing"? It's because traits are "baked in forever", we're having this discussion. Otherwise, none would bother belaboring. | |
| 14:27:28 | edleafe | efried: yep | |
| 14:27:46 | kashyap | aspiers: Yeah, stephenfin is only aggravating :D | |
| 14:27:50 | aspiers | kashyap is right about that, traits much need more care than most stuff | |
| 14:27:53 | efried | stephenfin: because there are traits that exist only on amd and only on intel. | |
| 14:28:03 | sean-k-mooney | stephenfin: well until openstack security ways in on the security question my current stance is we should add none of these tratis | |
| 14:28:27 | efried | From now on we shall require everyone to read all comments on this patch and pass a quiz before being allowed to participate in the conversation. | |
| 14:28:29 | aspiers | efried: I guess stephenfin's point was about which file they go in, not so much how they're namespaced | |
| 14:28:29 | kashyap | stephenfin: I know you have more than 10 minutes of attention span: have fun reading: https://review.opendev.org/#/c/655193/4 | |
| 14:28:44 | aspiers | efried: haha good idea! | |
| 14:29:00 | stephenfin | kashyap: Not really. I mean, they're still for x86 architectures, only those from specific companies | |
| 14:29:22 | stephenfin | Throwing everything in x86.py sounds a lot easier than trying to categorize each flag, IMO :) | |
| 14:29:37 | sean-k-mooney | stephenfin: i agree | |
| 14:29:50 | kashyap | sean-k-mooney: I think your stance there is invalid, recall what we discussed before: | |
| 14:29:53 | kashyap | < efried> kashyap, sean-k-mooney: I'm going to say it's fine to add the traits to os-traits regardless; the vulnerability occurs when we expose them via compute | |
| 14:30:16 | kashyap | stephenfin: Did you read the discussion here: https://review.opendev.org/#/c/655193/ ? | |
| 14:30:18 | stephenfin | I mean, we're not going to have an amd_intel.py file if a flag is only supported by AMD and Intel and not whoever else has an x86 license | |
| 14:30:33 | kashyap | (If not, there's more nuance than just following the IRC discussion here) | |
| 14:30:34 | stephenfin | kashyap: I did and held my tongue because I wan't sure | |
| 14:30:41 | kashyap | Heh | |
| 14:30:54 | efried | we have to categorize the intel- and amd-specific ones anyway. The strings that come out in the end are already figured. It's a question of whether to have INTEL_FOO and AMD_BAR in x86.py or have separate namespace dirs. | |
| 14:30:59 | sean-k-mooney | kashyap: my full stance on this is we should not do it peiord but if we must add these i would prefer them to be generic not vender specific and only as a last resouce expose vendor specific tratits | |
| 14:31:04 | stephenfin | but now that I've seen this, I've decided my first impression made some sense at least | |
| 14:31:43 | stephenfin | sean-k-mooney: Aye, agreed on the latter part of that at least. Not sure about the basis for the former so can't comment | |
| 14:32:10 | stephenfin | efried: Do we? Why? | |
| 14:32:19 | stephenfin | (just for context) | |
| 14:32:21 | kashyap | sean-k-mooney: You seem to be too hung up on 'generic'. It _doesn't_ fully make sense in this case. | |
| 14:32:39 | efried | I'll go out on a limb and say that anything x86 that's supported by both AMD and Intel should go into the x86 namespace, and random manufacturer that owns 0.0000001% of the x86 market who doesn't support that feature can just not turn it on. | |
| 14:33:03 | sean-k-mooney | kashyap: i think this is a missue use of traits and im pushing back becasue i dont think you have mad the case of why we should be exposing this in a non generic way | |
| 14:33:18 | efried | stephenfin: Because some of them are mutually exclusive. | |
| 14:33:20 | kashyap | sean-k-mooney: The case is the vulnerabilities cannot be trivially mapped into 'generic' traits> | |
| 14:33:33 | sean-k-mooney | traits are ment to be an abstration over hardware and are not intened to enable specifric feature flags in the emulated cpu | |
| 14:33:52 | efried | well now I can get behind that ^ | |
| 14:33:54 | kashyap | sean-k-mooney: And some people may want to run _without_ "L1TF" (which is Intel-only) | |
| 14:34:12 | stephenfin | efried: OK. How would that manifest itself in a way we care about? | |
| 14:34:19 | sean-k-mooney | kashyap: sure but why do this need to be in placement | |
| 14:34:25 | sean-k-mooney | as a trait | |
| 14:34:26 | stephenfin | (again, just so I know. Genuine question) | |
| 14:35:25 | sean-k-mooney | kashyap: my concern is that you appear to be trying to use tratis to carry vm configuration in fomation by making the vendor specific when we should be able to provide an abstration here | |
| 14:35:59 | efried | stephenfin: Apparently there's "has a flag you can turn on to mitigate X vulnerability" with variants of "do it on the CPU" and "do it in virt" and then there's "is manufactured without the vulnerability in the first place so no flags needed". And different variants of those exist depending on amd vs intel. | |
| 14:36:26 | kashyap | sean-k-mooney: No, you're assuming far too much. This whole discussion "exploded" when I simply noticed a few weeks ago some missing CPU traits. | |
| 14:36:39 | efried | stephenfin: I had already questioned whether we really cared about those distinctions, as opposed to "vulnerable or not". | |
| 14:36:45 | efried | apparently we do. | |
| 14:36:54 | sean-k-mooney | efried: right but the request to turn on the flag shoudl either be in the nova.conf or as a flavor extra spec or as an image proerty | |
| 14:36:56 | kashyap | And digigng more, I then thought: "If you have generic CPU flags listed, what about those that provide mitigation for security flaws.) | |
| 14:37:17 | sean-k-mooney | that flag should not be enabled by addign a required tratit to the flavor or image | |
| 14:37:24 | efried | sean-k-mooney: Right, but how do you schedule (or avoid scheduling to) a system that is or is not able to enable those flags? | |
| 14:37:46 | stephenfin | efried: OK, I wasn't aware of that. Thanks | |
| 14:37:56 | sean-k-mooney | we normalise the vendor diference in the virt dirver to a standard trait that is vendor independed | |
| 14:38:29 | kashyap | stephenfin: Your earlier question is answered in the commit message: "Notes on the "SSB"/"SSBD" confusion" | |
| 14:38:43 | kashyap | (Here: https://review.opendev.org/#/c/655193/) | |
| 14:39:10 | kashyap | (It notes how Intel and AMD addressed the same issue differently.) | |
| 14:39:19 | dansmith | kashyap: are you suggesting traits for flaws, not just traits for cpu flags? | |
| 14:40:26 | kashyap | dansmith: On "traits for flaws", I'm not sure if it makes sense. For now, all I care about is: all the required CPU flags are captured as traits | |
| 14:40:48 | kashyap | Err, s/required/missing/ | |
| 14:41:26 | sean-k-mooney | kashyap: efried suggested generic HW_CPU_HAS_MELTDOWN_CURE, HW_CPU_HAS_SPECTRE_CURE style traits in teh review | |
| 14:41:46 | kashyap | sean-k-mooney: And did you see my response that I already pasted here? | |
| 14:41:48 | sean-k-mooney | personally i think that is a better solution then what you are suggesting | |
| 14:41:48 | dansmith | :( | |
| 14:41:51 | artom | MELTDOWN_CURE sounds like a thing all parents of toddlers need | |
| 14:42:31 | kashyap | sean-k-mooney: Huh, you haven't read the response, that I posted, have you: | |
| 14:42:34 | kashyap | * The benefit of HW_CPU_HAS_SPECTRE_CURE is that it glosses over the | |
| 14:42:37 | kashyap | difference between the AMD and Intel flag names for the fix ('IBRS' vs | |
| 14:42:40 | kashyap | 'IBPB'). However, we likely going to want feature-based traits for | |
| 14:42:40 | sean-k-mooney | althoght i woudl personally go with hw_CPU_VULNERABLITY_SPECTER | |
| 14:42:42 | kashyap | Given the above it doesn't make sense the "generic roll-up traits". | |
| 14:42:42 | kashyap | other things: AES, PCID, etc. | |
| 14:42:45 | kashyap | * The HW_CPU_HAS_SPECTRE_CURE is ill-defined :-( since there are many | |
| 14:42:48 | kashyap | Spectre-related bugs. | |
| 14:43:01 | kashyap | s/"sense the"/"sense to have"/ | |