"struct segment_register" requires a prior inclusion of x86_emulate.h,
and it's missing a forward declaration of "struct vcpu" too.

Sort these out so the header can be included by itself.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
This dependency already exists today. I'm just making it explicit.

"segment_register" is weird. It naturally belongs in vmcb.h, but the
emulator makes use of it and must be compilable outside of Xen.

I don't like having vmcb.h depending on the emulator header, but I see
no way of breaking that dependency without breaking the emulator unit
tests.
---
 xen/arch/x86/include/asm/hvm/svm/vmcb.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/include/asm/hvm/svm/vmcb.h b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
index 28f715e376..7c599a1c39 100644
--- a/xen/arch/x86/include/asm/hvm/svm/vmcb.h
+++ b/xen/arch/x86/include/asm/hvm/svm/vmcb.h
@@ -10,6 +10,10 @@
 
 #include <xen/types.h>
 
+#include <asm/x86_emulate.h> /* for "struct segment_register" */
+
+struct vcpu;
+
 /* general 1 intercepts */
 enum GenericIntercept1bits
 {

base-commit: b6fc307b0b00314d4e4460fcf8be2cd9e4ff8652
-- 
2.43.0

On 06/10/2025 11:46 am, Alejandro Vallejo wrote:
> "struct segment_register" requires a prior inclusion of x86_emulate.h,
> and it's missing a forward declaration of "struct vcpu" too.
>
> Sort these out so the header can be included by itself.
>
> Not a functional change.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

I have a pending series for 4.22 which does substantially more
rearranging of this file and others.  See
https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/xen-svm-hdrs

I'd found the need for struct vcpu, but not for segment_register.  How
did you find it?

> ---
> This dependency already exists today. I'm just making it explicit.
>
> "segment_register" is weird. It naturally belongs in vmcb.h, but the
> emulator makes use of it and must be compilable outside of Xen.
>
> I don't like having vmcb.h depending on the emulator header, but I see
> no way of breaking that dependency without breaking the emulator unit
> tests.

Pulling it into a new header is fine.  The emulator can include anything
(free-enough standing) in arch/x86/include/asm/

~Andrew

On Mon Oct 6, 2025 at 1:04 PM CEST, Andrew Cooper wrote:
> On 06/10/2025 11:46 am, Alejandro Vallejo wrote:
>> "struct segment_register" requires a prior inclusion of x86_emulate.h,
>> and it's missing a forward declaration of "struct vcpu" too.
>>
>> Sort these out so the header can be included by itself.
>>
>> Not a functional change.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>
> I have a pending series for 4.22 which does substantially more
> rearranging of this file and others.  See
> https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=shortlog;h=refs/heads/xen-svm-hdrs
>
> I'd found the need for struct vcpu, but not for segment_register.  How
> did you find it?

By chance. I happened to include it in a file at a position where the
x86_emulate.h header hadn't been included.

>
>> ---
>> This dependency already exists today. I'm just making it explicit.
>>
>> "segment_register" is weird. It naturally belongs in vmcb.h, but the
>> emulator makes use of it and must be compilable outside of Xen.
>>
>> I don't like having vmcb.h depending on the emulator header, but I see
>> no way of breaking that dependency without breaking the emulator unit
>> tests.
>
> Pulling it into a new header is fine.  The emulator can include anything
> (free-enough standing) in arch/x86/include/asm/

I thought it didn't, if so I'm at a loss at to why segment_register is where
it is.

Be that as it may, your series conflicts with this patch and it makes no sense
to keep both around (when none of them are going in for until 4.22).

Will you be including a fix for the segment register in your series also? If so,
I'll just drop this one.

Cheers,
Alejandro

On 06/10/2025 12:29 pm, Alejandro Vallejo wrote:
> On Mon Oct 6, 2025 at 1:04 PM CEST, Andrew Cooper wrote:
>> On 06/10/2025 11:46 am, Alejandro Vallejo wrote:
>>> ---
>>> This dependency already exists today. I'm just making it explicit.
>>>
>>> "segment_register" is weird. It naturally belongs in vmcb.h, but the
>>> emulator makes use of it and must be compilable outside of Xen.
>>>
>>> I don't like having vmcb.h depending on the emulator header, but I see
>>> no way of breaking that dependency without breaking the emulator unit
>>> tests.
>> Pulling it into a new header is fine.  The emulator can include anything
>> (free-enough standing) in arch/x86/include/asm/
> I thought it didn't, if so I'm at a loss at to why segment_register is where
> it is.

Well - it's needed by the emulator as well as SVM.

>
> Be that as it may, your series conflicts with this patch and it makes no sense
> to keep both around (when none of them are going in for until 4.22).
>
> Will you be including a fix for the segment register in your series also? If so,
> I'll just drop this one.

I've folded the extra include into my first patch.

~Andrew

On Mon Oct 6, 2025 at 3:09 PM CEST, Andrew Cooper wrote:
> On 06/10/2025 12:29 pm, Alejandro Vallejo wrote:
>> On Mon Oct 6, 2025 at 1:04 PM CEST, Andrew Cooper wrote:
>>> On 06/10/2025 11:46 am, Alejandro Vallejo wrote:
>>>> ---
>>>> This dependency already exists today. I'm just making it explicit.
>>>>
>>>> "segment_register" is weird. It naturally belongs in vmcb.h, but the
>>>> emulator makes use of it and must be compilable outside of Xen.
>>>>
>>>> I don't like having vmcb.h depending on the emulator header, but I see
>>>> no way of breaking that dependency without breaking the emulator unit
>>>> tests.
>>> Pulling it into a new header is fine.  The emulator can include anything
>>> (free-enough standing) in arch/x86/include/asm/
>> I thought it didn't, if so I'm at a loss at to why segment_register is where
>> it is.
>
> Well - it's needed by the emulator as well as SVM.

I meant that it's an SVM-specific description of segments, which is then
conveniently reused on the emulator. It's a bit shocking for an SVM struct to
be described by the emulator.

I'd have expected the dependency to go the other way around, if it were possible
to have it that way.

>
>>
>> Be that as it may, your series conflicts with this patch and it makes no sense
>> to keep both around (when none of them are going in for until 4.22).
>>
>> Will you be including a fix for the segment register in your series also? If so,
>> I'll just drop this one.
>
> I've folded the extra include into my first patch.

Ta.

Cheers,
Alejandro