| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-14 | |||
| 14:18:59 | kashyap | Spectre-related bugs. | |
| 14:19:06 | kashyap | sean-k-mooney: Frankly, we're over-thinking on the generic "traits" thing. | |
| 14:19:21 | kashyap | I want to tackle that separately. And first get the granular traits in, without discussing this to the ends of the worlds. | |
| 14:19:25 | sean-k-mooney | kashyap: so traits shoudl not may to qemu feature flags directly | |
| 14:19:44 | mdbooth | stephenfin: I know you called it a nit, but given that I suspect it's a bug it wasn't caught by flake8 I'm going to respin anyway. Thanks for looking! | |
| 14:20:08 | sean-k-mooney | there is no reas an IBRS trait cant map to the IBPB feature on amd and IBRS on intel | |
| 14:20:14 | kashyap | sean-k-mooney: I disagree. | |
| 14:20:29 | stephenfin | mdbooth: Go for it. fwiw, I do have a series to update the hacking version we use but I've WIP'd that til the removal of cells v1 is complete | |
| 14:20:31 | sean-k-mooney | part of os-tratis is to provide a vendor independ set of common traits that can be normalised | |
| 14:20:37 | edleafe | efried: if we're going to be having _INTEL_* and _AMD_* traits, it would make more sense to have them in their own modules | |
| 14:20:38 | stephenfin | There's enough merge conflicts as it is | |
| 14:20:49 | sean-k-mooney | and them mapped to plathform specific thing in the virt dirvers | |
| 14:21:02 | kashyap | efried: Yeah, I'd like to know the answer to your earlier question | |
| 14:21:07 | kashyap | (On the structure) | |
| 14:21:56 | sean-k-mooney | edleafe: we dont really want to add tratis to the __init__.py file in general but we do allow it | |
| 14:22:11 | kashyap | Why we "don't really want"? | |
| 14:22:17 | kashyap | If efried says there's a precedence | |
| 14:22:22 | edleafe | sean-k-mooney: yeah, that's messier, but it does work | |
| 14:22:29 | edleafe | Separate modules is cleaner | |
| 14:22:41 | efried | sean-k-mooney: That's happening all over os-traits. In typical projects I agree it may be best avoided, but when in rome... | |
| 14:22:43 | sean-k-mooney | kashyap: because its condiered an anti pattern in python | |
| 14:22:58 | kashyap | Hm | |
| 14:23:27 | efried | of nine __init__.pyZ in os-traits, five have content. | |
| 14:23:38 | sean-k-mooney | efried: yes i know | |
| 14:23:49 | kashyap | sean-k-mooney: Also, look at: compute/__init__.py | |
| 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.) | |