> > > > In this case, given this is the common place, if we don't want to do > conditionally for different X86 architecture (may be not necessary), can you > put comments to clarify? > > This way, readers won't be confused. > > Will a single comment at the top of the function suffice? So, we are handling architecture-specific code in architecture-common path. Not sure how it will impact others. The idea solution will have to reshuffle it for different architecture to be processed differently. > > > > > > > > > > BTW, for those reserved bits, their meaning is not defined, and > > > > the VMM > > > should not depend on them IMO. > > > > What is the problem if hypervisor returns none-zero value? > > > > > > The problem arises if/when the bits become defined in the future, > > > and the functionality is not trivially virtualized. > > > > Assume the hardware defines the bit one day in future, if we are using old > VMM, the VMM will view the hardware as if the feature doesn't exist, since > VMM does not know the feature and view the bit as reserved. This case should > work. > > The VMM should be able to simply pass the results of > KVM_GET_SUPPORTED_CPUID to KVM_SET_CPUID2, without masking off bits > that it doesn't know about. At least in Intel architecture, a reserved bit (hardware return "1") reported by KVM_GET_SUPPORTED_CPUID can be set in KVM_SET_CPUID2. >From KVM p.o.v., I think we don't assume the reserved bit (set by KVM_SET_CPUID2) must be 0. Or I missed something? > > Nonetheless, a future VMM that does know about the new hardware feature > can certainly expect that a KVM that enumerates the feature in > KVM_GET_SUPPORTED_CPUID has the ability to support the feature. Taking native as example, a new application knows about the new hardware feature, should be able to run on old hardware without the feature, even if the old hardware report the reserved bit as "1". This is usually guaranteed by the hardware vendor in specification design. I will double check with Intel hardware guys to see if any "reserved bits" of CPUID may report "1". VMM in this context should be able to follow the principle too. > > > If we run with the future VMM, which recognizes and handles the > bit/feature. The VMM could view the hardware feature to be disabled / not > existed (if "1" is used to enable or stand for "having the capability"), or view > the hardware feature as enabled/ existed (if "0" is used to enable or stand for > "having the capability"). > > > > In this case, whether we have this patch doesn’t give us definite answer. Am > I right? > > Are you asking about the polarity of the CPUID bit? > > It is unfortunate that both Intel and AMD have started to define reverse > polarity CPUID bits, because these do, in fact, cause a forward compatibility > issue. Take, for example, AMD's recent introduction of > CPUID.80000008H:EBX.EferLmsleUnsupported[bit 20]. When the bit is set, > EFER.LMSLE is not available. For quite some time, KVM has been reporting > that this feature *is* available on CPUs where it isn't, because KVM cleared > the CPUID bit based on its previous 'reserved' definition. I have a series out to > address that issue: > https://lore.kernel.org/kvm/CALMp9eQ- > qkjBm8qPhOaMzZLWeHJcrwksV+XLQ9DfOQ_i1aykqQ@xxxxxxxxxxxxxx/. This case is different. It is because the new hardware converts a defined bit to be reserved. What we are talking here is to convert a reserved bit to defined bit. In this case, if one wants to remove the capability (if it is presented and run on old platform), to L1 VM. That is definitely fine. But extending this to vastly cover all reserved bits of CPUID leaf may be too aggressive, and cause the future issues. I will let you know if the reserved bits of CPUID in Intel architecture may report "1". If this is true, we may have to handle them differently for different architecture, i.e. Intel vs. AMD. Will that make sense? > > Intel did the same thing with > CPUID.(EAX=7H,ECX=0):EBX.FDP_EXCPTN_ONLY[bit 6] and > CPUID.(EAX=7H,ECX=0):EBX.ZERO_FCS_FDS[bit 13]. > > In the vast majority of cases, however, '0' is the right value to report for a > reserved bit to ensure forward compatibility. Thanks Eddie
