| Message ID | 20210303182219.1631042-1-philmd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | accel: Introduce AccelvCPUState opaque structure | expand |
On 03/03/21 19:22, Philippe Mathieu-Daudé wrote: > Series is organized as: > - preliminary trivial cleanups > - introduce AccelvCPUState > - move WHPX fields (build-tested) > - move HAX fields (not tested) > - move KVM fields (build-tested) > - move HVF fields (not tested) This approach prevents adding a TCG state. Have you thought of using a union instead, or even a void pointer? Paolo
On 3/4/21 2:56 PM, Paolo Bonzini wrote: > On 03/03/21 19:22, Philippe Mathieu-Daudé wrote: >> Series is organized as: >> - preliminary trivial cleanups >> - introduce AccelvCPUState >> - move WHPX fields (build-tested) >> - move HAX fields (not tested) >> - move KVM fields (build-tested) >> - move HVF fields (not tested) > > This approach prevents adding a TCG state. Have you thought of using a > union instead, or even a void pointer? Why does it prevent it? We can only have one accelerator per vCPU. TCG state has to be declared as another AccelvCPUState implementation. Am I missing something? Preventing building different accelerator-specific code in the same unit file is on purpose. Regards, Phil.
On 04/03/21 15:54, Philippe Mathieu-Daudé wrote: > On 3/4/21 2:56 PM, Paolo Bonzini wrote: >> On 03/03/21 19:22, Philippe Mathieu-Daudé wrote: >>> Series is organized as: >>> - preliminary trivial cleanups >>> - introduce AccelvCPUState >>> - move WHPX fields (build-tested) >>> - move HAX fields (not tested) >>> - move KVM fields (build-tested) >>> - move HVF fields (not tested) >> >> This approach prevents adding a TCG state. Have you thought of using a >> union instead, or even a void pointer? > > Why does it prevent it? We can only have one accelerator per vCPU. You're right, my misguided assumption was that there can only be one of WHPX/HAX/KVM/HVF. This is true for WHPX/KVM/HVF but HAX can live with any of the others. However this means that AccelvCPUState would have multiple definitions. Did you check that gdb copes well with it? It's also forbidden by C++[1], so another thing to check would be LTO when using the C++ compiler for linking. Paolo [1] https://en.wikipedia.org/wiki/One_Definition_Rule > TCG state has to be declared as another AccelvCPUState implementation. > > Am I missing something? > > Preventing building different accelerator-specific code in the same > unit file is on purpose. > > Regards, > > Phil. >
On 3/4/21 4:40 PM, Paolo Bonzini wrote: > On 04/03/21 15:54, Philippe Mathieu-Daudé wrote: >> On 3/4/21 2:56 PM, Paolo Bonzini wrote: >>> On 03/03/21 19:22, Philippe Mathieu-Daudé wrote: >>>> Series is organized as: >>>> - preliminary trivial cleanups >>>> - introduce AccelvCPUState >>>> - move WHPX fields (build-tested) >>>> - move HAX fields (not tested) >>>> - move KVM fields (build-tested) >>>> - move HVF fields (not tested) >>> >>> This approach prevents adding a TCG state. Have you thought of using a >>> union instead, or even a void pointer? >> >> Why does it prevent it? We can only have one accelerator per vCPU. > > You're right, my misguided assumption was that there can only be one of > WHPX/HAX/KVM/HVF. This is true for WHPX/KVM/HVF but HAX can live with > any of the others. I suppose you aren't talking about build-time but runtime. There should be no distinction related to accelerator at runtime. We should be able to have multiple accelerators at runtime, and eventually be able to migrate vCPU from one accelerator to another, if it is proven useful. How accelerators are orchestrated is obviously out of the scope of this series. > However this means that AccelvCPUState would have multiple definitions. Yes. > Did you check that gdb copes well with it? No, I haven't, because we already use opaque pointers elsewhere. > It's also forbidden by > C++[1], so another thing to check would be LTO when using the C++ > compiler for linking. OK, I have no clue about C++ (and tries to keep QEMU way from it) or about LTO. So I'd need to investigate that. > > Paolo > > [1] https://en.wikipedia.org/wiki/One_Definition_Rule > >> TCG state has to be declared as another AccelvCPUState implementation. >> >> Am I missing something? >> >> Preventing building different accelerator-specific code in the same >> unit file is on purpose. >> >> Regards, >> >> Phil. >> >